commit f5b2c7ae7404636337a726d3646976c240f861a5
author Spencer Low <CompareAndSwap@gmail.com> Thu Sep 17 19:36:10 2015 -0700
committer Yabin Cui <yabinc@google.com> Mon Sep 21 12:46:12 2015 -0700
tree ec7dcc3e0bfd272d5c2a4207e12703995206888c
parent 03b389085f38fc5f8b0902388b25bcb2ff0ffa99

base logging: fix errno restoring, severity conditionality, dangling ifs

Fix LOG() to properly save and restore errno. Test this properly.

Only do logging if severity is >= the minimum.

Fix dangling if statements in CHECK(), CHECK_STR{EQ,NE}(). Test this
properly.

Fix base logging tests on Windows. All libbase_tests now pass on
Windows.

Change place to lock, so the lock can protect logging of all data in
LogMessage.

Change-Id: I7ff531c67ae10a99ef0a2bbfe279aa77282d5ae9
Signed-off-by: Spencer Low <CompareAndSwap@gmail.com>

base/logging.cpp

diff --git a/base/logging.cpp b/base/logging.cpp
index e9e06df..6bfaaec 100644
--- a/base/logging.cpp
+++ b/base/logging.cpp
@@ -139,6 +139,10 @@
 static LogSeverity gMinimumLogSeverity = INFO;
 static std::unique_ptr<std::string> gProgramInvocationName;
 
+LogSeverity GetMinimumLogSeverity() {
+  return gMinimumLogSeverity;
+}
+
 static const char* ProgramInvocationName() {
   if (gProgramInvocationName == nullptr) {
     gProgramInvocationName.reset(new std::string(getprogname()));
@@ -200,20 +204,6 @@
   InitLogging(argv);
 }
 
-// TODO: make this public; it's independently useful.
-class ErrnoRestorer {
- public:
-  ErrnoRestorer(int saved_errno) : saved_errno_(saved_errno) {
-  }
-
-  ~ErrnoRestorer() {
-    errno = saved_errno_;
-  }
-
- private:
-  const int saved_errno_;
-};
-
 void InitLogging(char* argv[]) {
   if (gInitialized) {
     return;
@@ -286,13 +276,12 @@
 class LogMessageData {
  public:
   LogMessageData(const char* file, unsigned int line, LogId id,
-                 LogSeverity severity, int error, int saved_errno)
+                 LogSeverity severity, int error)
       : file_(GetFileBasename(file)),
         line_number_(line),
         id_(id),
         severity_(severity),
-        error_(error),
-        errno_restorer_(saved_errno) {
+        error_(error) {
   }
 
   const char* GetFile() const {
@@ -330,39 +319,38 @@
   const LogId id_;
   const LogSeverity severity_;
   const int error_;
-  ErrnoRestorer errno_restorer_;
 
   DISALLOW_COPY_AND_ASSIGN(LogMessageData);
 };
 
 LogMessage::LogMessage(const char* file, unsigned int line, LogId id,
                        LogSeverity severity, int error)
-    : data_(new LogMessageData(file, line, id, severity, error, errno)) {
+    : data_(new LogMessageData(file, line, id, severity, error)) {
 }
 
 LogMessage::~LogMessage() {
-  if (data_->GetSeverity() < gMinimumLogSeverity) {
-    return;  // No need to format something we're not going to output.
-  }
-
   // Finish constructing the message.
   if (data_->GetError() != -1) {
     data_->GetBuffer() << ": " << strerror(data_->GetError());
   }
   std::string msg(data_->ToString());
 
-  if (msg.find('\n') == std::string::npos) {
-    LogLine(data_->GetFile(), data_->GetLineNumber(), data_->GetId(),
-            data_->GetSeverity(), msg.c_str());
-  } else {
-    msg += '\n';
-    size_t i = 0;
-    while (i < msg.size()) {
-      size_t nl = msg.find('\n', i);
-      msg[nl] = '\0';
+  {
+    // Do the actual logging with the lock held.
+    lock_guard<mutex> lock(logging_lock);
+    if (msg.find('\n') == std::string::npos) {
       LogLine(data_->GetFile(), data_->GetLineNumber(), data_->GetId(),
-              data_->GetSeverity(), &msg[i]);
-      i = nl + 1;
+              data_->GetSeverity(), msg.c_str());
+    } else {
+      msg += '\n';
+      size_t i = 0;
+      while (i < msg.size()) {
+        size_t nl = msg.find('\n', i);
+        msg[nl] = '\0';
+        LogLine(data_->GetFile(), data_->GetLineNumber(), data_->GetId(),
+                data_->GetSeverity(), &msg[i]);
+        i = nl + 1;
+      }
     }
   }
 
@@ -382,7 +370,6 @@
 void LogMessage::LogLine(const char* file, unsigned int line, LogId id,
                          LogSeverity severity, const char* message) {
   const char* tag = ProgramInvocationName();
-  lock_guard<mutex> lock(logging_lock);
   gLogger(id, severity, tag, file, line, message);
 }