From 4b885c8633f2f47ceca04e341d7f771cf9b8e224 Mon Sep 17 00:00:00 2001 From: Victor Zverovich Date: Thu, 6 May 2021 09:20:46 -0700 Subject: [PATCH] Replace windows_error with system_error --- .github/workflows/linux.yml | 2 + .github/workflows/macos.yml | 2 + .github/workflows/windows.yml | 2 + include/fmt/os.h | 70 +++++++++++++++++------------------ src/os.cc | 11 ++---- test/os-test.cc | 47 +++++++++++------------ test/posix-mock-test.cc | 12 ++++-- 7 files changed, 76 insertions(+), 70 deletions(-) diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 200c8bb0..4c9794bc 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -70,3 +70,5 @@ jobs: - name: Test working-directory: ${{runner.workspace}}/build run: ctest -C ${{matrix.build_type}} + env: + CTEST_OUTPUT_ON_FAILURE: True diff --git a/.github/workflows/macos.yml b/.github/workflows/macos.yml index 33be5005..2fc67afb 100644 --- a/.github/workflows/macos.yml +++ b/.github/workflows/macos.yml @@ -28,3 +28,5 @@ jobs: - name: Test working-directory: ${{runner.workspace}}/build run: ctest -C ${{matrix.build_type}} + env: + CTEST_OUTPUT_ON_FAILURE: True diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index b405c4ed..b2ad28be 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -54,3 +54,5 @@ jobs: - name: Test working-directory: ${{runner.workspace}}/build run: ctest -C ${{matrix.build_type}} + env: + CTEST_OUTPUT_ON_FAILURE: True diff --git a/include/fmt/os.h b/include/fmt/os.h index 14c8c8b6..7d9166b0 100644 --- a/include/fmt/os.h +++ b/include/fmt/os.h @@ -9,10 +9,11 @@ #define FMT_OS_H_ #include -#include // for locale_t +#include // locale_t #include #include -#include // for strtod_l +#include // strtod_l +#include // std::system_error #if defined __APPLE__ || defined(__FreeBSD__) # include // for LC_NUMERIC_MASK on OS X @@ -154,45 +155,42 @@ FMT_API void format_windows_error(buffer& out, int error_code, string_view message) FMT_NOEXCEPT; } // namespace detail -/** A Windows error. */ -class windows_error : public system_error { - private: - FMT_API void init(int error_code, string_view format_str, format_args args); +FMT_API std::system_error vwindows_error(int error_code, string_view format_str, + format_args args); - public: - /** - \rst - Constructs a :class:`fmt::windows_error` object with the description - of the form +/** + \rst + Constructs a :class:`std::system_error` object with the description + of the form - .. parsed-literal:: - **: ** + .. parsed-literal:: + **: ** - where ** is the formatted message and ** is the - system message corresponding to the error code. - *error_code* is a Windows error code as given by ``GetLastError``. - If *error_code* is not a valid error code such as -1, the system message - will look like "error -1". + where ** is the formatted message and ** is the + system message corresponding to the error code. + *error_code* is a Windows error code as given by ``GetLastError``. + If *error_code* is not a valid error code such as -1, the system message + will look like "error -1". - **Example**:: + **Example**:: - // This throws a windows_error with the description - // cannot open file 'madeup': The system cannot find the file specified. - // or similar (system message may vary). - const char *filename = "madeup"; - LPOFSTRUCT of = LPOFSTRUCT(); - HFILE file = OpenFile(filename, &of, OF_READ); - if (file == HFILE_ERROR) { - throw fmt::windows_error(GetLastError(), - "cannot open file '{}'", filename); - } - \endrst - */ - template - windows_error(int error_code, string_view message, const Args&... args) { - init(error_code, message, make_format_args(args...)); - } -}; + // This throws a windows_error with the description + // cannot open file 'madeup': The system cannot find the file specified. + // or similar (system message may vary). + const char *filename = "madeup"; + LPOFSTRUCT of = LPOFSTRUCT(); + HFILE file = OpenFile(filename, &of, OF_READ); + if (file == HFILE_ERROR) { + throw fmt::windows_error(GetLastError(), + "cannot open file '{}'", filename); + } + \endrst +*/ +template +std::system_error windows_error(int error_code, string_view message, + const Args&... args) { + return vwindows_error(error_code, message, make_format_args(args...)); +} // Reports a Windows error without throwing an exception. // Can be used to report errors from destructors. diff --git a/src/os.cc b/src/os.cc index 42a88b32..b55f61af 100644 --- a/src/os.cc +++ b/src/os.cc @@ -100,13 +100,10 @@ int detail::utf16_to_utf8::convert(wstring_view s) { return 0; } -void windows_error::init(int err_code, string_view format_str, - format_args args) { - error_code_ = err_code; - memory_buffer buffer; - detail::format_windows_error(buffer, err_code, vformat(format_str, args)); - std::runtime_error& base = *this; - base = std::runtime_error(to_string(buffer)); +std::system_error vwindows_error(int err_code, string_view format_str, + format_args args) { + auto ec = std::error_code(err_code, std::system_category()); + throw std::system_error(ec, vformat(format_str, args)); } void detail::format_windows_error(detail::buffer& out, int error_code, diff --git a/test/os-test.cc b/test/os-test.cc index 91abac63..589f8fb2 100644 --- a/test/os-test.cc +++ b/test/os-test.cc @@ -20,6 +20,7 @@ using fmt::buffered_file; using fmt::error_code; +using testing::HasSubstr; #ifdef _WIN32 @@ -45,14 +46,15 @@ void check_utf_conversion_error( fmt::basic_string_view str = fmt::basic_string_view(0, 1)) { fmt::memory_buffer out; fmt::detail::format_windows_error(out, ERROR_INVALID_PARAMETER, message); - auto error = fmt::system_error(0, ""); + out.resize(out.size() - 2); // Remove newline. + auto error = std::system_error(std::error_code()); try { (Converter)(str); - } catch (const fmt::system_error& e) { + } catch (const std::system_error& e) { error = e; } - EXPECT_EQ(ERROR_INVALID_PARAMETER, error.error_code()); - EXPECT_EQ(fmt::to_string(out), error.what()); + EXPECT_EQ(ERROR_INVALID_PARAMETER, error.code().value()); + EXPECT_THAT(error.what(), HasSubstr(fmt::to_string(out))); } TEST(util_test, utf16_to_utf8_error) { @@ -69,11 +71,11 @@ TEST(util_test, utf16_to_utf8_convert) { TEST(os_test, format_windows_error) { LPWSTR message = 0; - FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | - FORMAT_MESSAGE_IGNORE_INSERTS, - 0, ERROR_FILE_EXISTS, - MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), - reinterpret_cast(&message), 0, 0); + auto result = FormatMessageW( + FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS, + 0, ERROR_FILE_EXISTS, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + reinterpret_cast(&message), 0, 0); fmt::detail::utf16_to_utf8 utf8_message(message); LocalFree(message); fmt::memory_buffer actual_message; @@ -93,16 +95,14 @@ TEST(os_test, format_long_windows_error) { // this error code is not available on all Windows platforms and // Windows SDKs, so do not fail the test if the error string cannot // be retrieved. - const int provisioning_not_allowed = - 0x80284013L /*TBS_E_PROVISIONING_NOT_ALLOWED*/; - if (FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | - FORMAT_MESSAGE_FROM_SYSTEM | - FORMAT_MESSAGE_IGNORE_INSERTS, - 0, static_cast(provisioning_not_allowed), - MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), - reinterpret_cast(&message), 0, 0) == 0) { - return; - } + int provisioning_not_allowed = 0x80284013L; // TBS_E_PROVISIONING_NOT_ALLOWED + auto result = FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER | + FORMAT_MESSAGE_FROM_SYSTEM | + FORMAT_MESSAGE_IGNORE_INSERTS, + 0, static_cast(provisioning_not_allowed), + MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), + reinterpret_cast(&message), 0, 0); + EXPECT_NE(result, 0); fmt::detail::utf16_to_utf8 utf8_message(message); LocalFree(message); fmt::memory_buffer actual_message; @@ -113,16 +113,17 @@ TEST(os_test, format_long_windows_error) { } TEST(os_test, windows_error) { - auto error = fmt::system_error(0, ""); + auto error = std::system_error(std::error_code()); try { throw fmt::windows_error(ERROR_FILE_EXISTS, "test {}", "error"); - } catch (const fmt::system_error& e) { + } catch (const std::system_error& e) { error = e; } fmt::memory_buffer message; fmt::detail::format_windows_error(message, ERROR_FILE_EXISTS, "test error"); - EXPECT_EQ(to_string(message), error.what()); - EXPECT_EQ(ERROR_FILE_EXISTS, error.error_code()); + message.resize(message.size() - 2); + EXPECT_THAT(error.what(), HasSubstr(to_string(message))); + EXPECT_EQ(ERROR_FILE_EXISTS, error.code().value()); } TEST(os_test, report_windows_error) { diff --git a/test/posix-mock-test.cc b/test/posix-mock-test.cc index dd3aaac6..19ad2753 100644 --- a/test/posix-mock-test.cc +++ b/test/posix-mock-test.cc @@ -264,12 +264,16 @@ TEST(file_test, size) { EXPECT_GE(f.size(), 0); EXPECT_EQ(content.size(), static_cast(f.size())); # ifdef _WIN32 - fmt::memory_buffer message; - fmt::detail::format_windows_error(message, ERROR_ACCESS_DENIED, - "cannot get file size"); + auto error_code = std::error_code(); fstat_sim = error; - EXPECT_THROW_MSG(f.size(), fmt::windows_error, fmt::to_string(message)); + try { + f.size(); + } catch (const std::system_error& e) { + error_code = e.code(); + } fstat_sim = none; + EXPECT_EQ(error_code, + std::error_code(ERROR_ACCESS_DENIED, std::system_category())); # else f.close(); EXPECT_SYSTEM_ERROR(f.size(), EBADF, "cannot get file attributes");