Fix issue #2274.

fmtlib · May 9, 2021 · b6c5d40 · b6c5d40
1 parent 2a9b314
commit b6c5d40
Show file tree

Hide file tree

Showing 4 changed files with 79 additions and 26 deletions.
diff --git a/include/fmt/os.h b/include/fmt/os.h
@@ -135,6 +135,8 @@ template <typename Char> struct formatter<std::error_code, Char> {
   }
 };
 
+FMT_API const std::error_category& system_category() FMT_NOEXCEPT;
+
 #ifdef _WIN32
 namespace detail {
 // A converter from UTF-16 to UTF-8.

diff --git a/src/os.cc b/src/os.cc
@@ -100,35 +100,79 @@ int detail::utf16_to_utf8::convert(wstring_view s) {
   return 0;
 }
 
+namespace detail {
+
+class system_message {
+  system_message(const system_message&) = delete;
+  void operator=(const system_message&) = delete;
+
+  unsigned long result_;
+  wchar_t* message_;
+
+  static bool is_whitespace(wchar_t c) FMT_NOEXCEPT {
+    return c == L' ' || c == L'\n' || c == L'\r' || c == L'\t' || c == L'\0';
+  }
+
+ public:
+  system_message(unsigned long error_code) : result_(0), message_(nullptr) {
+    result_ = FormatMessageW(
+        FORMAT_MESSAGE_ALLOCATE_BUFFER | FORMAT_MESSAGE_FROM_SYSTEM |
+            FORMAT_MESSAGE_IGNORE_INSERTS,
+        nullptr, error_code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+        reinterpret_cast<wchar_t*>(&message_), 0, nullptr);
+  }
+  ~system_message() { LocalFree(message_); }
+  unsigned long result() const FMT_NOEXCEPT { return result_; }
+  operator wstring_view() const FMT_NOEXCEPT {
+    return wstring_view(message_, result_);
+  }
+  wstring_view rtrim() const FMT_NOEXCEPT {
+    auto len = result_;
+    while (len != 0 && is_whitespace(message_[len - 1])) {
+      --len;
+    }
+    return wstring_view(message_, len);
+  }
+};
+
+class utf8_system_category final : public std::error_category {
+ public:
+  const char* name() const FMT_NOEXCEPT override { return "system"; }
+  std::string message(int error_code) const override {
+    system_message msg(error_code);
+    if (msg.result() != 0) {
+      utf16_to_utf8 utf8_message;
+      if (utf8_message.convert(msg.rtrim()) == ERROR_SUCCESS) {
+        return utf8_message.str();
+      }
+    }
+    return "unknown error";
+  }
+};
+
+}  // namespace detail
+
+FMT_API const std::error_category& system_category() FMT_NOEXCEPT {
+  static const detail::utf8_system_category category;
+  return category;
+}
+
 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());
+  auto ec = std::error_code(err_code, system_category());
   throw std::system_error(ec, vformat(format_str, args));
 }
 
 void detail::format_windows_error(detail::buffer<char>& out, int error_code,
                                   const char* message) FMT_NOEXCEPT {
   FMT_TRY {
-    wmemory_buffer buf;
-    buf.resize(inline_buffer_size);
-    for (;;) {
-      wchar_t* system_message = &buf[0];
-      int result = FormatMessageW(
-          FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, nullptr,
-          error_code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), system_message,
-          static_cast<uint32_t>(buf.size()), nullptr);
-      if (result != 0) {
-        utf16_to_utf8 utf8_message;
-        if (utf8_message.convert(system_message) == ERROR_SUCCESS) {
-          format_to(buffer_appender<char>(out), "{}: {}", message,
-                    utf8_message);
-          return;
-        }
-        break;
+    system_message msg(error_code);
+    if (msg.result() != 0) {
+      utf16_to_utf8 utf8_message;
+      if (utf8_message.convert(msg) == ERROR_SUCCESS) {
+        format_to(buffer_appender<char>(out), "{}: {}", message, utf8_message);
+        return;
       }
-      if (GetLastError() != ERROR_INSUFFICIENT_BUFFER)
-        break;  // Can't get error message, report error code instead.
-      buf.resize(buf.size() * 2);
     }
   }
   FMT_CATCH(...) {}
@@ -138,6 +182,10 @@ void detail::format_windows_error(detail::buffer<char>& out, int error_code,
 void report_windows_error(int error_code, const char* message) FMT_NOEXCEPT {
   report_error(detail::format_windows_error, error_code, message);
 }
+#else
+const std::error_category& system_category() FMT_NOEXCEPT {
+  return std::system_category();
+}
 #endif  // _WIN32
 
 buffered_file::~buffered_file() FMT_NOEXCEPT {

diff --git a/test/os-test.cc b/test/os-test.cc
@@ -74,10 +74,10 @@ TEST(os_test, format_std_error_code) {
                         std::error_code(42, std::generic_category())));
   EXPECT_EQ("system:42",
             fmt::format(FMT_STRING("{0}"),
-                        std::error_code(42, std::system_category())));
+                        std::error_code(42, fmt::system_category())));
   EXPECT_EQ("system:-42",
             fmt::format(FMT_STRING("{0}"),
-                        std::error_code(-42, std::system_category())));
+                        std::error_code(-42, fmt::system_category())));
 }
 
 TEST(os_test, format_std_error_code_wide) {
@@ -86,10 +86,10 @@ TEST(os_test, format_std_error_code_wide) {
                         std::error_code(42, std::generic_category())));
   EXPECT_EQ(L"system:42",
             fmt::format(FMT_STRING(L"{0}"),
-                        std::error_code(42, std::system_category())));
+                        std::error_code(42, fmt::system_category())));
   EXPECT_EQ(L"system:-42",
             fmt::format(FMT_STRING(L"{0}"),
-                        std::error_code(-42, std::system_category())));
+                        std::error_code(-42, fmt::system_category())));
 }
 
 TEST(os_test, format_windows_error) {
@@ -120,7 +120,10 @@ TEST(os_test, format_long_windows_error) {
                                0, static_cast<DWORD>(provisioning_not_allowed),
                                MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
                                reinterpret_cast<LPWSTR>(&message), 0, 0);
-  EXPECT_NE(result, 0);
+  if (result == 0) {
+    LocalFree(message);
+    return;
+  }
   fmt::detail::utf16_to_utf8 utf8_message(message);
   LocalFree(message);
   fmt::memory_buffer actual_message;

diff --git a/test/posix-mock-test.cc b/test/posix-mock-test.cc
@@ -272,7 +272,7 @@ TEST(file_test, size) {
   }
   fstat_sim = none;
   EXPECT_EQ(error_code,
-            std::error_code(ERROR_ACCESS_DENIED, std::system_category()));
+            std::error_code(ERROR_ACCESS_DENIED, fmt::system_category()));
 #  else
   f.close();
   EXPECT_SYSTEM_ERROR(f.size(), EBADF, "cannot get file attributes");