Fix StrError, move Format*ErrorMessage to the implementation.

format-test.cc

@@ -250,9 +250,10 @@ TEST(UtilTest, UTF8ToUTF16) {
 
 TEST(UtilTest, StrError) {
   using fmt::internal::StrError;
-  EXPECT_DEBUG_DEATH(StrError(EDOM, 0, 0), "Assertion");
+  char *message = 0;
+  EXPECT_DEBUG_DEATH(StrError(EDOM, message = 0, 0), "Assertion");
   char buffer[BUFFER_SIZE];
-  EXPECT_DEBUG_DEATH(StrError(EDOM, buffer, 0), "Assertion");
+  EXPECT_DEBUG_DEATH(StrError(EDOM, message = buffer, 0), "Assertion");
   buffer[0] = 'x';
 #ifdef _GNU_SOURCE
   // Use invalid error code to make sure that StrError returns an error
@@ -261,16 +262,16 @@ TEST(UtilTest, StrError) {
 #else
   int error_code = EDOM;
 #endif
-  const char *message = StrError(error_code, buffer, 1);
+  int result = StrError(error_code, message = buffer, 1);
   EXPECT_EQ(buffer, message);  // Message should point to buffer.
-  EXPECT_EQ(ERANGE, errno);
+  EXPECT_EQ(ERANGE, result);
   EXPECT_STREQ("", message);
-  message = StrError(error_code, buffer, BUFFER_SIZE);
+  result = StrError(error_code, message = buffer, BUFFER_SIZE);
-  EXPECT_EQ(0, errno);
+  EXPECT_EQ(0, result);
   EXPECT_GE(BUFFER_SIZE - 1, std::strlen(message));
   EXPECT_STREQ(strerror(error_code), message);
-  message = StrError(error_code, buffer, std::strlen(message));
+  result = StrError(error_code, message = buffer, std::strlen(message));
-  EXPECT_EQ(ERANGE, errno);
+  EXPECT_EQ(ERANGE, result);
 }
 
 TEST(UtilTest, SystemError) {
@@ -1651,6 +1652,8 @@ TEST(FormatterTest, FileSinkWriteError) {
   std::fclose(f);
 }
 
+// TODO: test SystemErrorSink, ThrowSystemError, CErrorSink, ThrowCError.
+
 // The test doesn't compile on older compilers which follow C++03 and
 // require an accessible copy constructor when binding a temporary to
 // a const reference.

format.cc

@@ -100,6 +100,58 @@ inline int FMT_SNPRINTF(char *buffer, size_t size, const char *format, ...) {
 #endif  // _MSC_VER
 
 const char RESET_COLOR[] = "\x1b[0m";
+
+void FormatCErrorMessage(
+    fmt::Writer &out, int error_code, fmt::StringRef message) {
+  fmt::internal::Array<char, fmt::internal::INLINE_BUFFER_SIZE> buffer;
+  buffer.resize(fmt::internal::INLINE_BUFFER_SIZE);
+  char *system_message = 0;
+  for (;;) {
+    system_message = &buffer[0];
+    int result = fmt::internal::StrError(
+        error_code, system_message, buffer.size());
+    if (result == 0)
+      break;
+    if (result != ERANGE) {
+      // Can't get error message, report error code instead.
+      out << message << ": error code = " << error_code;
+      return;
+    }
+    buffer.resize(buffer.size() * 2);
+  }
+  out << message << ": " << system_message;
+}
+
+void FormatSystemErrorMessage(
+    fmt::Writer &out, int error_code, fmt::StringRef message) {
+#ifndef _WIN32
+  FormatCErrorMessage(out, error_code, message);
+#else
+  class String {
+   private:
+    LPWSTR str_;
+
+   public:
+    String() : str_() {}
+    ~String() { LocalFree(str_); }
+    LPWSTR *ptr() { return &str_; }
+    LPCWSTR c_str() const { return str_; }
+  };
+  String system_message;
+  if (FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER |
+      FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, 0,
+      error_code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
+      reinterpret_cast<LPWSTR>(system_message.ptr()), 0, 0)) {
+    UTF16ToUTF8 utf8_message;
+    if (!utf8_message.Convert(system_message.c_str())) {
+      out << message << ": " << c_str(utf8_message);
+      return;
+    }
+  }
+  // Can't get error message, report error code instead.
+  out << message << ": error code = " << error_code;
+#endif
+}
 }
 
 template <typename T>
@@ -204,74 +256,27 @@ int fmt::internal::UTF16ToUTF8::Convert(fmt::WStringRef s) {
 
 #endif
 
-char *fmt::internal::StrError(
+int fmt::internal::StrError(
-    int error_code, char *buffer, std::size_t buffer_size) {
+    int error_code, char *&buffer, std::size_t buffer_size) {
   assert(buffer != 0 && buffer_size != 0);
-  errno = 0;
+  int result = 0;
 #ifdef _GNU_SOURCE
   char *message = strerror_r(error_code, buffer, buffer_size);
+  // If the buffer is full then the message is probably truncated.
   if (message == buffer && strlen(buffer) == buffer_size - 1)
-    errno = ERANGE;  // The buffer is full so the message is probably truncated.
+    result = ERANGE;
-  return message;
+  buffer = message;
 #elif _WIN32
-  errno = strerror_s(buffer, buffer_size, error_code);
+  result = strerror_s(buffer, buffer_size, error_code);
-  if (errno == 0 && std::strlen(buffer) == buffer_size - 1)
+  // If the buffer is full then the message is probably truncated.
-    errno = ERANGE;  // The buffer is full so the message is probably truncated.
+  if (result == 0 && std::strlen(buffer) == buffer_size - 1)
-  return buffer;
+    result = ERANGE;
 #else
-  strerror_r(error_code, buffer, buffer_size);
+  result = strerror_r(error_code, buffer, buffer_size);
-  return buffer;
+  if (result == -1)
-#endif
+    result = errno;  // glibc versions before 2.13 return result in errno.
-}
-
-void fmt::internal::FormatCErrorMessage(
-    fmt::Writer &out, int error_code, fmt::StringRef message) {
-  Array<char, INLINE_BUFFER_SIZE> buffer;
-  buffer.resize(INLINE_BUFFER_SIZE);
-  char *system_message = 0;
-  for (;;) {
-    system_message = StrError(error_code, &buffer[0], buffer.size());
-    if (errno == 0)
-      break;
-    if (errno != ERANGE) {
-      // Can't get error message, report error code instead.
-      out << message << ": error code = " << error_code;
-      return;
-    }
-    buffer.resize(buffer.size() * 2);
-  }
-  out << message << ": " << system_message;
-}
-
-void fmt::internal::FormatSystemErrorMessage(
-    fmt::Writer &out, int error_code, fmt::StringRef message) {
-#ifndef _WIN32
-  FormatCErrorMessage(out, error_code, message);
-#else
-  class String {
-   private:
-    LPWSTR str_;
-
-   public:
-    String() : str_() {}
-    ~String() { LocalFree(str_); }
-    LPWSTR *ptr() { return &str_; }
-    LPCWSTR c_str() const { return str_; }
-  };
-  String system_message;
-  if (FormatMessageW(FORMAT_MESSAGE_ALLOCATE_BUFFER |
-      FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS, 0,
-      error_code, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT),
-      reinterpret_cast<LPWSTR>(system_message.ptr()), 0, 0)) {
-    UTF16ToUTF8 utf8_message;
-    if (!utf8_message.Convert(system_message.c_str())) {
-      out << message << ": " << c_str(utf8_message);
-      return;
-    }
-  }
-  // Can't get error message, report error code instead.
-  out << message << ": error code = " << error_code;
 #endif
+  return result;
 }
 
 // Fills the padding around the content and returns the pointer to the
@@ -802,13 +807,13 @@ void fmt::BasicWriter<Char>::FormatParser::Format(
 
 void fmt::SystemErrorSink::operator()(const fmt::Writer &w) const {
   Writer message;
-  internal::FormatSystemErrorMessage(message, error_code_, w.c_str());
+  FormatSystemErrorMessage(message, error_code_, w.c_str());
   throw SystemError(message.c_str(), error_code_);
 }
 
 void fmt::CErrorSink::operator()(const Writer &w) const {
   Writer message;
-  internal::FormatCErrorMessage(message, error_code_, w.c_str());
+  FormatCErrorMessage(message, error_code_, w.c_str());
   throw SystemError(message.c_str(), error_code_);
 }
 

format.h

@@ -490,20 +490,15 @@ class UTF16ToUTF8 {
 #endif
 
 // Portable thread-safe version of strerror.
-// Returns a pointer to a string describing the error code. This can be
+// Sets buffer to point to a string describing the error code.
-// either a pointer to a string stored in buffer, or a pointer to some
+// This can be either a pointer to a string stored in buffer,
-// static immutable string. Sets errno to one of the following values:
+// or a pointer to some static immutable string.
+// Returns one of the following values:
 //   0      - success
 //   ERANGE - buffer is not large enough to store the error message
 //   other  - failure
 // Buffer should be at least of size 1.
-char *StrError(int error_code, char *buffer, std::size_t buffer_size);
+int StrError(int error_code, char *&buffer, std::size_t buffer_size);
-
-// Formats a standard C library error message writing the output to out.
-void FormatCErrorMessage(Writer &out, int error_code, StringRef message);
-
-// Formats a system error message writing the output to out.
-void FormatSystemErrorMessage(Writer &out, int error_code, StringRef message);
 
 }  // namespace internal