Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print errno and strerror in POSIX transport to see better error messages #2373

Merged
merged 1 commit into from
Jul 13, 2020
Merged

Print errno and strerror in POSIX transport to see better error messages #2373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 47 additions & 18 deletions source/adios2/toolkit/transport/file/FilePOSIX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
#include "FilePOSIX.h"

#include <cstdio> // remove
#include <cstring> // strerror
#include <errno.h> // errno
#include <fcntl.h> // open
#include <stddef.h> // write output
#include <sys/stat.h> // open, fstat
Expand Down Expand Up @@ -47,9 +49,7 @@ void FilePOSIX::WaitForOpen()
m_FileDescriptor = m_OpenFuture.get();
}
m_IsOpening = false;
CheckFile(
"couldn't open file " + m_Name +
", check permissions or path existence, in call to POSIX open");
CheckFile("couldn't open file " + m_Name + ", in call to POSIX open");
m_IsOpen = true;
}
}
Expand All @@ -59,7 +59,9 @@ void FilePOSIX::Open(const std::string &name, const Mode openMode,
{
auto lf_AsyncOpenWrite = [&](const std::string &name) -> int {
ProfilerStart("open");
errno = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused about this. You set errno to zero, then assign m_Errno to errno, without any intervening assignments. So m_Errno is always zero.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow I just read about errno; I guess it behaves like an eflags register.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an intervening system function call that sets 'errno' to non-zero if there is an error. It's standard practice (for C programmers at least) to zero out errno before system calls to not catch errors from the past, hence the errno=0 statement here. m_Errno is used to remember the (possible) error for the call right after this and checked later (possibly the call in happens in a thread, line 63, but the check is in main thread much later)

int FD = open(m_Name.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0666);
m_Errno = errno;
ProfilerStop("open");
return FD;
};
Expand All @@ -80,23 +82,29 @@ void FilePOSIX::Open(const std::string &name, const Mode openMode,
else
{
ProfilerStart("open");
errno = 0;
m_FileDescriptor =
open(m_Name.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0666);
m_Errno = errno;
ProfilerStop("open");
}
break;

case (Mode::Append):
ProfilerStart("open");
errno = 0;
// m_FileDescriptor = open(m_Name.c_str(), O_RDWR);
m_FileDescriptor = open(m_Name.c_str(), O_RDWR | O_CREAT, 0777);
lseek(m_FileDescriptor, 0, SEEK_END);
m_Errno = errno;
ProfilerStop("open");
break;

case (Mode::Read):
ProfilerStart("open");
errno = 0;
m_FileDescriptor = open(m_Name.c_str(), O_RDONLY);
m_Errno = errno;
ProfilerStop("open");
break;

Expand All @@ -107,9 +115,7 @@ void FilePOSIX::Open(const std::string &name, const Mode openMode,

if (!m_IsOpening)
{
CheckFile(
"couldn't open file " + m_Name +
", check permissions or path existence, in call to POSIX open");
CheckFile("couldn't open file " + m_Name + ", in call to POSIX open");
m_IsOpen = true;
}
}
Expand All @@ -120,7 +126,9 @@ void FilePOSIX::Write(const char *buffer, size_t size, size_t start)
while (size > 0)
{
ProfilerStart("write");
errno = 0;
const auto writtenSize = write(m_FileDescriptor, buffer, size);
m_Errno = errno;
ProfilerStop("write");

if (writtenSize == -1)
Expand All @@ -132,7 +140,7 @@ void FilePOSIX::Write(const char *buffer, size_t size, size_t start)

throw std::ios_base::failure(
"ERROR: couldn't write to file " + m_Name +
", in call to FileDescriptor Write\n");
", in call to POSIX Write" + SysErrMsg());
}

buffer += writtenSize;
Expand All @@ -143,14 +151,16 @@ void FilePOSIX::Write(const char *buffer, size_t size, size_t start)
WaitForOpen();
if (start != MaxSizeT)
{
errno = 0;
const auto newPosition = lseek(m_FileDescriptor, start, SEEK_SET);
m_Errno = errno;

if (static_cast<size_t>(newPosition) != start)
{
throw std::ios_base::failure(
"ERROR: couldn't move to start position " +
std::to_string(start) + " in file " + m_Name +
", in call to POSIX lseek\n");
", in call to POSIX lseek" + SysErrMsg());
}
}

Expand Down Expand Up @@ -179,7 +189,9 @@ void FilePOSIX::Read(char *buffer, size_t size, size_t start)
while (size > 0)
{
ProfilerStart("read");
errno = 0;
const auto readSize = read(m_FileDescriptor, buffer, size);
m_Errno = errno;
ProfilerStop("read");

if (readSize == -1)
Expand All @@ -189,9 +201,9 @@ void FilePOSIX::Read(char *buffer, size_t size, size_t start)
continue;
}

throw std::ios_base::failure("ERROR: couldn't read from file " +
m_Name +
", in call to POSIX IO read\n");
throw std::ios_base::failure(
"ERROR: couldn't read from file " + m_Name +
", in call to POSIX IO read" + SysErrMsg());
}

buffer += readSize;
Expand All @@ -203,15 +215,16 @@ void FilePOSIX::Read(char *buffer, size_t size, size_t start)

if (start != MaxSizeT)
{
errno = 0;
const auto newPosition = lseek(m_FileDescriptor, start, SEEK_SET);
m_Errno = errno;

if (static_cast<size_t>(newPosition) != start)
{
throw std::ios_base::failure(
"ERROR: couldn't move to start position " +
std::to_string(start) + " in file " + m_Name +
", in call to POSIX lseek errno " + std::to_string(errno) +
"\n");
", in call to POSIX lseek" + SysErrMsg());
}
}

Expand All @@ -238,11 +251,14 @@ size_t FilePOSIX::GetSize()
{
struct stat fileStat;
WaitForOpen();
errno = 0;
if (fstat(m_FileDescriptor, &fileStat) == -1)
{
m_Errno = errno;
throw std::ios_base::failure("ERROR: couldn't get size of file " +
m_Name + "\n");
m_Name + SysErrMsg());
}
m_Errno = errno;
return static_cast<size_t>(fileStat.st_size);
}

Expand All @@ -252,13 +268,16 @@ void FilePOSIX::Close()
{
WaitForOpen();
ProfilerStart("close");
errno = 0;
const int status = close(m_FileDescriptor);
m_Errno = errno;
ProfilerStop("close");

if (status == -1)
{
throw std::ios_base::failure("ERROR: couldn't close file " + m_Name +
", in call to POSIX IO close\n");
", in call to POSIX IO close" +
SysErrMsg());
}

m_IsOpen = false;
Expand All @@ -278,31 +297,41 @@ void FilePOSIX::CheckFile(const std::string hint) const
{
if (m_FileDescriptor == -1)
{
throw std::ios_base::failure("ERROR: " + hint + "\n");
throw std::ios_base::failure("ERROR: " + hint + SysErrMsg());
}
}

std::string FilePOSIX::SysErrMsg() const
{
return std::string(": errno = " + std::to_string(m_Errno) + ": " +
strerror(m_Errno));
}

void FilePOSIX::SeekToEnd()
{
WaitForOpen();
errno = 0;
const int status = lseek(m_FileDescriptor, 0, SEEK_END);
m_Errno = 0;
if (status == -1)
{
throw std::ios_base::failure(
"ERROR: couldn't seek to the end of file " + m_Name +
", in call to POSIX IO lseek\n");
", in call to POSIX IO lseek" + SysErrMsg());
}
}

void FilePOSIX::SeekToBegin()
{
WaitForOpen();
errno = 0;
const int status = lseek(m_FileDescriptor, 0, SEEK_SET);
m_Errno = errno;
if (status == -1)
{
throw std::ios_base::failure(
"ERROR: couldn't seek to the begin of file " + m_Name +
", in call to POSIX IO lseek\n");
", in call to POSIX IO lseek" + SysErrMsg());
}
}

Expand Down
2 changes: 2 additions & 0 deletions source/adios2/toolkit/transport/file/FilePOSIX.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class FilePOSIX : public Transport
private:
/** POSIX file handle returned by Open */
int m_FileDescriptor = -1;
int m_Errno = 0;
bool m_IsOpening = false;
std::future<int> m_OpenFuture;

Expand All @@ -66,6 +67,7 @@ class FilePOSIX : public Transport
*/
void CheckFile(const std::string hint) const;
void WaitForOpen();
std::string SysErrMsg() const;
};

} // end namespace transport
Expand Down