Extracted logging::SystemErrorCodeToString function.

Allows to use any errors, and particularly useful for logging HRESULT on Windows.

Included error code on windows into error message.

Removed unused LOG_GETLASTERROR_MODULE logging macros.

Review URL: https://codereview.chromium.org/244583002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@265460 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 48 insertions, 87 deletions

diff --git a/base/logging.cc b/base/logging.cc
index 5475615..9fe235c 100644
--- a/base/logging.cc
+++ b/base/logging.cc
@@ -43,6 +43,7 @@ typedef pthread_mutex_t* MutexHandle;
 #include <ctime>
 #include <iomanip>
 #include <ostream>
+#include <string>
 
 #include "base/base_switches.h"
 #include "base/command_line.h"
@@ -51,6 +52,8 @@ typedef pthread_mutex_t* MutexHandle;
 #include "base/debug/stack_trace.h"
 #include "base/posix/eintr_wrapper.h"
 #include "base/strings/string_piece.h"
+#include "base/strings/string_util.h"
+#include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/synchronization/lock_impl.h"
 #include "base/threading/platform_thread.h"
@@ -729,61 +732,40 @@ SystemErrorCode GetLastSystemErrorCode() {
 }
 
 #if defined(OS_WIN)
-Win32ErrorLogMessage::Win32ErrorLogMessage(const char* file,
-                                           int line,
-                                           LogSeverity severity,
-                                           SystemErrorCode err,
-                                           const char* module)
-    : err_(err),
-      module_(module),
-      log_message_(file, line, severity) {
+BASE_EXPORT std::string SystemErrorCodeToString(SystemErrorCode error_code) {
+  const int error_message_buffer_size = 256;
+  char msgbuf[error_message_buffer_size];
+  DWORD flags = FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS;
+  DWORD len = FormatMessageA(flags, NULL, error_code, 0, msgbuf,
+                             arraysize(msgbuf), NULL);
+  if (len) {
+    // Messages returned by system end with line breaks.
+    return base::CollapseWhitespaceASCII(msgbuf, true) +
+        base::StringPrintf(" (0x%X)", error_code);
+  }
+  return base::StringPrintf("Error (0x%X) while retrieving error. (0x%X)",
+                            GetLastError(), error_code);
 }
+#elif defined(OS_POSIX)
+BASE_EXPORT std::string SystemErrorCodeToString(SystemErrorCode error_code) {
+  return safe_strerror(error_code);
+}
+#else
+#error Not implemented
+#endif
+
 
+#if defined(OS_WIN)
 Win32ErrorLogMessage::Win32ErrorLogMessage(const char* file,
                                            int line,
                                            LogSeverity severity,
                                            SystemErrorCode err)
     : err_(err),
-      module_(NULL),
       log_message_(file, line, severity) {
 }
 
 Win32ErrorLogMessage::~Win32ErrorLogMessage() {
-  const int error_message_buffer_size = 256;
-  char msgbuf[error_message_buffer_size];
-  DWORD flags = FORMAT_MESSAGE_FROM_SYSTEM | FORMAT_MESSAGE_IGNORE_INSERTS;
-  HMODULE hmod;
-  if (module_) {
-    hmod = GetModuleHandleA(module_);
-    if (hmod) {
-      flags |= FORMAT_MESSAGE_FROM_HMODULE;
-    } else {
-      // This makes a nested Win32ErrorLogMessage. It will have module_ of NULL
-      // so it will not call GetModuleHandle, so recursive errors are
-      // impossible.
-      DPLOG(WARNING) << "Couldn't open module " << module_
-          << " for error message query";
-    }
-  } else {
-    hmod = NULL;
-  }
-  DWORD len = FormatMessageA(flags,
-                             hmod,
-                             err_,
-                             0,
-                             msgbuf,
-                             sizeof(msgbuf) / sizeof(msgbuf[0]),
-                             NULL);
-  if (len) {
-    while ((len > 0) &&
-           isspace(static_cast<unsigned char>(msgbuf[len - 1]))) {
-      msgbuf[--len] = 0;
-    }
-    stream() << ": " << msgbuf;
-  } else {
-    stream() << ": Error " << GetLastError() << " while retrieving error "
-        << err_;
-  }
+  stream() << ": " << SystemErrorCodeToString(err_);
   // We're about to crash (CHECK). Put |err_| on the stack (by placing it in a
   // field) and use Alias in hopes that it makes it into crash dumps.
   DWORD last_error = err_;
@@ -799,7 +781,7 @@ ErrnoLogMessage::ErrnoLogMessage(const char* file,
 }
 
 ErrnoLogMessage::~ErrnoLogMessage() {
-  stream() << ": " << safe_strerror(err_);
+  stream() << ": " << SystemErrorCodeToString(err_);
 }
 #endif  // OS_WIN
 
diff --git a/base/logging.h b/base/logging.h
index 8a268fa..dd17bce 100644
--- a/base/logging.h
+++ b/base/logging.h
@@ -426,29 +426,13 @@ const LogSeverity LOG_0 = LOG_ERROR;
   SYSLOG_IF(FATAL, !(condition)) << "Assert failed: " #condition ". "
 
 #if defined(OS_WIN)
-#define LOG_GETLASTERROR_STREAM(severity) \
+#define PLOG_STREAM(severity) \
   COMPACT_GOOGLE_LOG_EX_ ## severity(Win32ErrorLogMessage, \
       ::logging::GetLastSystemErrorCode()).stream()
-#define LOG_GETLASTERROR(severity) \
-  LAZY_STREAM(LOG_GETLASTERROR_STREAM(severity), LOG_IS_ON(severity))
-#define LOG_GETLASTERROR_MODULE_STREAM(severity, module) \
-  COMPACT_GOOGLE_LOG_EX_ ## severity(Win32ErrorLogMessage, \
-      ::logging::GetLastSystemErrorCode(), module).stream()
-#define LOG_GETLASTERROR_MODULE(severity, module)                       \
-  LAZY_STREAM(LOG_GETLASTERROR_STREAM(severity, module),                \
-              LOG_IS_ON(severity))
-// PLOG_STREAM is used by PLOG, which is the usual error logging macro
-// for each platform.
-#define PLOG_STREAM(severity) LOG_GETLASTERROR_STREAM(severity)
 #elif defined(OS_POSIX)
-#define LOG_ERRNO_STREAM(severity) \
+#define PLOG_STREAM(severity) \
   COMPACT_GOOGLE_LOG_EX_ ## severity(ErrnoLogMessage, \
       ::logging::GetLastSystemErrorCode()).stream()
-#define LOG_ERRNO(severity) \
-  LAZY_STREAM(LOG_ERRNO_STREAM(severity), LOG_IS_ON(severity))
-// PLOG_STREAM is used by PLOG, which is the usual error logging macro
-// for each platform.
-#define PLOG_STREAM(severity) LOG_ERRNO_STREAM(severity)
 #endif
 
 #define PLOG(severity)                                          \
@@ -622,17 +606,6 @@ enum { DEBUG_MODE = ENABLE_DLOG };
 #define DLOG(severity)                                          \
   LAZY_STREAM(LOG_STREAM(severity), DLOG_IS_ON(severity))
 
-#if defined(OS_WIN)
-#define DLOG_GETLASTERROR(severity) \
-  LAZY_STREAM(LOG_GETLASTERROR_STREAM(severity), DLOG_IS_ON(severity))
-#define DLOG_GETLASTERROR_MODULE(severity, module)                      \
-  LAZY_STREAM(LOG_GETLASTERROR_STREAM(severity, module),                \
-              DLOG_IS_ON(severity))
-#elif defined(OS_POSIX)
-#define DLOG_ERRNO(severity)                                    \
-  LAZY_STREAM(LOG_ERRNO_STREAM(severity), DLOG_IS_ON(severity))
-#endif
-
 #define DPLOG(severity)                                         \
   LAZY_STREAM(PLOG_STREAM(severity), DLOG_IS_ON(severity))
 
@@ -640,6 +613,15 @@ enum { DEBUG_MODE = ENABLE_DLOG };
 
 #define DVPLOG(verboselevel) DVPLOG_IF(verboselevel, VLOG_IS_ON(verboselevel))
 
+// TODO(vitalybuka): following should be removed and replaced with PLOG.
+#if defined(OS_WIN)
+#define LOG_GETLASTERROR(severity) PLOG(severity)
+#define DLOG_GETLASTERROR(severity) DPLOG(severity)
+#elif defined(OS_POSIX)
+#define LOG_ERRNO(severity) PLOG(severity)
+#define DLOG_ERRNO(severity) DPLOG(severity)
+#endif
+
 // Definitions for DCHECK et al.
 
 #if DCHECK_IS_ON
@@ -823,6 +805,7 @@ typedef int SystemErrorCode;
 // Alias for ::GetLastError() on Windows and errno on POSIX. Avoids having to
 // pull in windows.h just for GetLastError() and DWORD.
 BASE_EXPORT SystemErrorCode GetLastSystemErrorCode();
+BASE_EXPORT std::string SystemErrorCodeToString(SystemErrorCode error_code);
 
 #if defined(OS_WIN)
 // Appends a formatted system message of the GetLastError() type.
@@ -831,12 +814,6 @@ class BASE_EXPORT Win32ErrorLogMessage {
   Win32ErrorLogMessage(const char* file,
                        int line,
                        LogSeverity severity,
-                       SystemErrorCode err,
-                       const char* module);
-
-  Win32ErrorLogMessage(const char* file,
-                       int line,
-                       LogSeverity severity,
                        SystemErrorCode err);
 
   // Appends the error message before destructing the encapsulated class.
@@ -846,8 +823,6 @@ class BASE_EXPORT Win32ErrorLogMessage {
 
  private:
   SystemErrorCode err_;
-  // Optional name of the module defining the error.
-  const char* module_;
   LogMessage log_message_;
 
   DISALLOW_COPY_AND_ASSIGN(Win32ErrorLogMessage);
diff --git a/base/logging_unittest.cc b/base/logging_unittest.cc
index d03ed93..44a1be3 100644
--- a/base/logging_unittest.cc
+++ b/base/logging_unittest.cc
@@ -54,11 +54,7 @@ class MockLogSource {
 
 TEST_F(LoggingTest, BasicLogging) {
   MockLogSource mock_log_source;
-  const int kExpectedDebugOrReleaseCalls = 6;
-  const int kExpectedDebugCalls = 6;
-  const int kExpectedCalls =
-      kExpectedDebugOrReleaseCalls + (DEBUG_MODE ? kExpectedDebugCalls : 0);
-  EXPECT_CALL(mock_log_source, Log()).Times(kExpectedCalls).
+  EXPECT_CALL(mock_log_source, Log()).Times(DEBUG_MODE ? 16 : 8).
       WillRepeatedly(Return("log message"));
 
   SetMinLogLevel(LOG_INFO);
@@ -76,6 +72,8 @@ TEST_F(LoggingTest, BasicLogging) {
   PLOG_IF(INFO, true) << mock_log_source.Log();
   VLOG(0) << mock_log_source.Log();
   VLOG_IF(0, true) << mock_log_source.Log();
+  VPLOG(0) << mock_log_source.Log();
+  VPLOG_IF(0, true) << mock_log_source.Log();
 
   DLOG(INFO) << mock_log_source.Log();
   DLOG_IF(INFO, true) << mock_log_source.Log();
@@ -83,6 +81,8 @@ TEST_F(LoggingTest, BasicLogging) {
   DPLOG_IF(INFO, true) << mock_log_source.Log();
   DVLOG(0) << mock_log_source.Log();
   DVLOG_IF(0, true) << mock_log_source.Log();
+  DVPLOG(0) << mock_log_source.Log();
+  DVPLOG_IF(0, true) << mock_log_source.Log();
 }
 
 TEST_F(LoggingTest, LogIsOn) {
@@ -159,6 +159,8 @@ TEST_F(LoggingTest, LoggingIsLazy) {
   PLOG_IF(INFO, false) << mock_log_source.Log();
   VLOG(1) << mock_log_source.Log();
   VLOG_IF(1, true) << mock_log_source.Log();
+  VPLOG(1) << mock_log_source.Log();
+  VPLOG_IF(1, true) << mock_log_source.Log();
 
   DLOG(INFO) << mock_log_source.Log();
   DLOG_IF(INFO, true) << mock_log_source.Log();
@@ -166,6 +168,8 @@ TEST_F(LoggingTest, LoggingIsLazy) {
   DPLOG_IF(INFO, true) << mock_log_source.Log();
   DVLOG(1) << mock_log_source.Log();
   DVLOG_IF(1, true) << mock_log_source.Log();
+  DVPLOG(1) << mock_log_source.Log();
+  DVPLOG_IF(1, true) << mock_log_source.Log();
 }
 
 // Official builds have CHECKs directly call BreakDebugger.