leveldb: Clean up chromium_logger.h and port_chromium.{cc,h}.

This CL updates the chromium_logger.h implementation with the changes in
https://github.com/google/leveldb/commit/7b945f200339aa47c24788d3ee9910c09c513843.

The CL also does minor cleanup in the Chromium port code.

Change-Id: I6ec4a464963bd0011ee01ea9879fda5d8836418e
Reviewed-on: https://chromium-review.googlesource.com/1205639Reviewed-by: Chris Mumford <cmumford@chromium.org>
Commit-Queue: Victor Costan <pwnall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590441}

third_party/leveldatabase/chromium_logger.h

@@ -5,79 +5,102 @@
 #ifndef THIRD_PARTY_LEVELDATABASE_CHROMIUM_LOGGER_H_
 #define THIRD_PARTY_LEVELDATABASE_CHROMIUM_LOGGER_H_
 
-#include <stdint.h>
-
+#include <cstdarg>
 #include <utility>
 
 #include "base/files/file.h"
 #include "base/format_macros.h"
+#include "base/strings/string_util.h"
+#include "base/threading/platform_thread.h"
 #include "base/time/time.h"
 
 namespace leveldb {
 
 class ChromiumLogger : public Logger {
  public:
-  explicit ChromiumLogger(base::File f) : file_(std::move(f)) {}
-  virtual ~ChromiumLogger() {}
-  virtual void Logv(const char* format, va_list ap) {
-    const base::PlatformThreadId thread_id = base::PlatformThread::CurrentId();
+  explicit ChromiumLogger(base::File file) : file_(std::move(file)) {}
 
-    // We try twice: the first time with a fixed-size stack allocated buffer,
-    // and the second time with a much larger dynamically allocated buffer.
-    char buffer[500];
-    for (int iter = 0; iter < 2; iter++) {
-      char* base;
-      int bufsize;
-      if (iter == 0) {
-        bufsize = sizeof(buffer);
-        base = buffer;
-      } else {
-        bufsize = 30000;
-        base = new char[bufsize];
-      }
-      char* p = base;
-      char* limit = base + bufsize;
-
-      base::Time::Exploded t;
-      base::Time::Now().LocalExplode(&t);
-
-      p += base::snprintf(p, limit - p,
-                    "%04d/%02d/%02d-%02d:%02d:%02d.%03d %" PRIu64 " ",
-                    t.year,
-                    t.month,
-                    t.day_of_month,
-                    t.hour,
-                    t.minute,
-                    t.second,
-                    t.millisecond,
-                    static_cast<uint64_t>(thread_id));
-
-      // Print the message
-      if (p < limit) {
-        va_list backup_ap;
-        va_copy(backup_ap, ap);
-        p += vsnprintf(p, limit - p, format, backup_ap);
-        va_end(backup_ap);
-      }
+  ~ChromiumLogger() override = default;
+
+  void Logv(const char* format, va_list arguments) override {
+    // Record the time as close to the Logv() call as possible.
+    base::Time::Exploded now_exploded;
+    base::Time::Now().LocalExplode(&now_exploded);
 
-      // Truncate to available space if necessary
-      if (p >= limit) {
-        if (iter == 0) {
-          continue;       // Try again with larger buffer
-        } else {
-          p = limit - 1;
+    const base::PlatformThreadId thread_id = base::PlatformThread::CurrentId();
+
+    // We first attempt to print into a stack-allocated buffer. If this attempt
+    // fails, we make a second attempt with a dynamically allocated buffer.
+    constexpr const int kStackBufferSize = 512;
+    char stack_buffer[kStackBufferSize];
+    static_assert(sizeof(stack_buffer) == static_cast<size_t>(kStackBufferSize),
+                  "sizeof(char) is expected to be 1 in C++");
+
+    int dynamic_buffer_size = 0;  // Computed in the first iteration.
+    for (int iteration = 0; iteration < 2; ++iteration) {
+      const int buffer_size =
+          (iteration == 0) ? kStackBufferSize : dynamic_buffer_size;
+      char* const buffer =
+          (iteration == 0) ? stack_buffer : new char[dynamic_buffer_size];
+
+      // Print the header into the buffer.
+      int buffer_offset = base::snprintf(
+          buffer, buffer_size,
+          "%04d/%02d/%02d-%02d:%02d:%02d.%03d %" PRIx64 " ",
+          now_exploded.year,
+          now_exploded.month,
+          now_exploded.day_of_month,
+          now_exploded.hour,
+          now_exploded.minute,
+          now_exploded.second,
+          now_exploded.millisecond,
+          static_cast<uint64_t>(thread_id));
+
+      // The header can be at most 45 characters (10 date + 12 time + 3 spacing
+      // + 20 thread ID), which should fit comfortably into the static buffer.
+      DCHECK_LE(buffer_offset, 45);
+      static_assert(45 < kStackBufferSize,
+                    "stack-allocated buffer may not fit the message header");
+      DCHECK_LT(buffer_offset, buffer_size);
+
+      // Print the message into the buffer.
+      std::va_list arguments_copy;
+      va_copy(arguments_copy, arguments);
+      buffer_offset += std::vsnprintf(buffer + buffer_offset,
+                                      buffer_size - buffer_offset, format,
+                                      arguments_copy);
+      va_end(arguments_copy);
+
+      // The code below may append a newline at the end of the buffer, which
+      // requires an extra character.
+      if (buffer_offset >= buffer_size - 1) {
+        // The message did not fit into the buffer.
+        if (iteration == 0) {
+          // Re-run the loop and use a dynamically-allocated buffer. The buffer
+          // will be large enough for the log message, an extra newline and a
+          // null terminator.
+          dynamic_buffer_size = buffer_offset + 2;
+          continue;
         }
+
+        // The dynamically-allocated buffer was incorrectly sized. This should
+        // not happen, assuming a correct implementation of (v)snprintf. Fail
+        // in tests, recover by truncating the log message in production.
+        NOTREACHED();
+        buffer_offset = buffer_size - 1;
       }
 
-      // Add newline if necessary
-      if (p == base || p[-1] != '\n') {
-        *p++ = '\n';
+      // Add a newline if necessary.
+      if (buffer[buffer_offset - 1] != '\n') {
+        buffer[buffer_offset] = '\n';
+        ++buffer_offset;
       }
 
-      assert(p <= limit);
-      file_.WriteAtCurrentPos(base, p - base);
-      if (base != buffer) {
-        delete[] base;
+      DCHECK_LE(buffer_offset, buffer_size);
+      file_.WriteAtCurrentPos(buffer, buffer_offset);
+
+      if (iteration != 0) {
+        delete[] buffer;
       }
       break;
     }

third_party/leveldatabase/port/port_chromium.cc

@@ -14,11 +14,9 @@
 namespace leveldb {
 namespace port {
 
-Mutex::Mutex() {
-}
+Mutex::Mutex() = default;
 
-Mutex::~Mutex() {
-}
+Mutex::~Mutex() = default;
 
 void Mutex::Lock() {
   mu_.Acquire();
@@ -36,7 +34,7 @@ CondVar::CondVar(Mutex* mu)
     : cv_(&mu->mu_) {
 }
 
-CondVar::~CondVar() { }
+CondVar::~CondVar() = default;
 
 void CondVar::Wait() {
   cv_.Wait();

third_party/leveldatabase/port/port_chromium.h

@@ -61,12 +61,12 @@ class CondVar {
 };
 
 class AtomicPointer {
- private:
-  typedef base::subtle::AtomicWord Rep;
-  Rep rep_;
  public:
-  AtomicPointer() { }
+  AtomicPointer() = default;
+  ~AtomicPointer() = default;
+
   explicit AtomicPointer(void* p) : rep_(reinterpret_cast<Rep>(p)) {}
+
   inline void* Acquire_Load() const {
     return reinterpret_cast<void*>(base::subtle::Acquire_Load(&rep_));
   }
@@ -79,6 +79,10 @@ class AtomicPointer {
   inline void NoBarrier_Store(void* v) {
     base::subtle::NoBarrier_Store(&rep_, reinterpret_cast<Rep>(v));
   }
+
+ private:
+  using Rep = base::subtle::AtomicWord;
+  Rep rep_;
 };
 
 // Implementation of OnceType and InitOnce() pair, this is equivalent to