From c95a4432d7c9d0e5829cd802908700ba2e2c25dc Mon Sep 17 00:00:00 2001 From: John Moffett Date: Mon, 5 Dec 2022 12:59:15 -0500 Subject: [PATCH 1/2] Show descriptive error messages when FileCommit fails Only raw errno codes are logged if FileCommit fails. These are implementation-specific, so it makes it harder to debug based on user reports. Instead, use SysErrorString to display both the raw int value and the descriptive message. --- src/util/fs_helpers.cpp | 11 ++++++----- src/util/sock.cpp | 13 +------------ src/util/syserror.cpp | 24 ++++++++++++++++++++++++ src/util/syserror.h | 4 ++++ 4 files changed, 35 insertions(+), 17 deletions(-) diff --git a/src/util/fs_helpers.cpp b/src/util/fs_helpers.cpp index d05cb8a63d..2a9eb3502e 100644 --- a/src/util/fs_helpers.cpp +++ b/src/util/fs_helpers.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -120,28 +121,28 @@ std::streampos GetFileSize(const char* path, std::streamsize max) bool FileCommit(FILE* file) { if (fflush(file) != 0) { // harmless if redundantly called - LogPrintf("%s: fflush failed: %d\n", __func__, errno); + LogPrintf("fflush failed: %s\n", SysErrorString(errno)); return false; } #ifdef WIN32 HANDLE hFile = (HANDLE)_get_osfhandle(_fileno(file)); if (FlushFileBuffers(hFile) == 0) { - LogPrintf("%s: FlushFileBuffers failed: %d\n", __func__, GetLastError()); + LogPrintf("FlushFileBuffers failed: %s\n", Win32ErrorString(GetLastError())); return false; } #elif defined(MAC_OSX) && defined(F_FULLFSYNC) if (fcntl(fileno(file), F_FULLFSYNC, 0) == -1) { // Manpage says "value other than -1" is returned on success - LogPrintf("%s: fcntl F_FULLFSYNC failed: %d\n", __func__, errno); + LogPrintf("fcntl F_FULLFSYNC failed: %s\n", SysErrorString(errno)); return false; } #elif HAVE_FDATASYNC if (fdatasync(fileno(file)) != 0 && errno != EINVAL) { // Ignore EINVAL for filesystems that don't support sync - LogPrintf("%s: fdatasync failed: %d\n", __func__, errno); + LogPrintf("fdatasync failed: %s\n", SysErrorString(errno)); return false; } #else if (fsync(fileno(file)) != 0 && errno != EINVAL) { - LogPrintf("%s: fsync failed: %d\n", __func__, errno); + LogPrintf("fsync failed: %s\n", SysErrorString(errno)); return false; } #endif diff --git a/src/util/sock.cpp b/src/util/sock.cpp index 53d20bdf19..d33ccd135e 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -419,18 +419,7 @@ void Sock::Close() #ifdef WIN32 std::string NetworkErrorString(int err) { - wchar_t buf[256]; - buf[0] = 0; - if(FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_MAX_WIDTH_MASK, - nullptr, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), - buf, ARRAYSIZE(buf), nullptr)) - { - return strprintf("%s (%d)", std::wstring_convert,wchar_t>().to_bytes(buf), err); - } - else - { - return strprintf("Unknown error (%d)", err); - } + return Win32ErrorString(err); } #else std::string NetworkErrorString(int err) diff --git a/src/util/syserror.cpp b/src/util/syserror.cpp index 5270f55366..bac498a23d 100644 --- a/src/util/syserror.cpp +++ b/src/util/syserror.cpp @@ -12,6 +12,12 @@ #include #include +#if defined(WIN32) +#include +#include +#include +#endif + std::string SysErrorString(int err) { char buf[1024]; @@ -33,3 +39,21 @@ std::string SysErrorString(int err) return strprintf("Unknown error (%d)", err); } } + +#if defined(WIN32) +std::string Win32ErrorString(int err) +{ + wchar_t buf[256]; + buf[0] = 0; + if(FormatMessageW(FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS | FORMAT_MESSAGE_MAX_WIDTH_MASK, + nullptr, err, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + buf, ARRAYSIZE(buf), nullptr)) + { + return strprintf("%s (%d)", std::wstring_convert,wchar_t>().to_bytes(buf), err); + } + else + { + return strprintf("Unknown error (%d)", err); + } +} +#endif diff --git a/src/util/syserror.h b/src/util/syserror.h index a54ba553ee..eb02141b9d 100644 --- a/src/util/syserror.h +++ b/src/util/syserror.h @@ -13,4 +13,8 @@ */ std::string SysErrorString(int err); +#if defined(WIN32) +std::string Win32ErrorString(int err); +#endif + #endif // BITCOIN_UTIL_SYSERROR_H From 5408a55fc87350baeae3a32f1003f956d5533a79 Mon Sep 17 00:00:00 2001 From: John Moffett Date: Thu, 8 Dec 2022 13:45:51 -0500 Subject: [PATCH 2/2] Consolidate Win32-specific error formatting GetErrorReason()'s Win32 implementation does the same thing as Win32ErrorString(int err) from syserror.cpp, so call the latter. Also remove now-unnecessary headers from sock.cpp and less verbose handling of #ifdefs. --- src/util/fs.cpp | 7 +------ src/util/sock.cpp | 12 ++---------- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/src/util/fs.cpp b/src/util/fs.cpp index e8fb72670f..14f7a44661 100644 --- a/src/util/fs.cpp +++ b/src/util/fs.cpp @@ -81,12 +81,7 @@ bool FileLock::TryLock() #else static std::string GetErrorReason() { - wchar_t* err; - FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, - nullptr, GetLastError(), MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), reinterpret_cast(&err), 0, nullptr); - std::wstring err_str(err); - LocalFree(err); - return std::wstring_convert>().to_bytes(err_str); + return Win32ErrorString(GetLastError()); } FileLock::FileLock(const fs::path& file) diff --git a/src/util/sock.cpp b/src/util/sock.cpp index d33ccd135e..f244f38f3f 100644 --- a/src/util/sock.cpp +++ b/src/util/sock.cpp @@ -15,11 +15,6 @@ #include #include -#ifdef WIN32 -#include -#include -#endif - #ifdef USE_POLL #include #endif @@ -416,15 +411,12 @@ void Sock::Close() m_socket = INVALID_SOCKET; } -#ifdef WIN32 std::string NetworkErrorString(int err) { +#if defined(WIN32) return Win32ErrorString(err); -} #else -std::string NetworkErrorString(int err) -{ // On BSD sockets implementations, NetworkErrorString is the same as SysErrorString. return SysErrorString(err); -} #endif +}