Merge in changes from readability review.

src/log.cc

Index: src/log.cc
diff --git a/src/log.cc b/src/log.cc
index 3d3cd2d262b9604502283bf10eee1db34026e977..8fbbee391a702a3839b296f0561c6ebfc42b5be0 100644
--- a/src/log.cc
+++ b/src/log.cc
@@ -301,30 +301,37 @@ class Log : public AllStatic {
   // Frees all resources acquired in Open... functions.
   static void Close();
 
-  // See description in v8.h.
+  // See description in include/v8.h.
   static int GetLogLines(int from_pos, char* dest_buf, int max_size);
 
-  static bool is_enabled() { return output_.handle != NULL; }
+  // Returns whether logging is enabled.
+  static bool is_enabled() {

[Søren Thygesen Gjesse, 2009/05/25 06:38:31]

How about changing this to IsEnabled() as well? This is no really an accessor.

[Mikhail Naganov, 2009/05/25 08:38:51]

Good idea. Done.

+    return output_handle_ != NULL || output_buffer_ != NULL;
+  }
 
-  typedef int (*WritePtr)(const char* msg, int length);
  private:
+  typedef int (*WritePtr)(const char* msg, int length);
+
+  // Initialization function called from Open... functions.
   static void Init();
 
   // Write functions assume that mutex_ is acquired by the caller.
   static WritePtr Write;
 
+  // Implementation of writing to a log file.
   static int WriteToFile(const char* msg, int length) {
-    ASSERT(output_.handle != NULL);
-    int rv = fwrite(msg, 1, length, output_.handle);
+    ASSERT(output_handle_ != NULL);
+    int rv = fwrite(msg, 1, length, output_handle_);
     ASSERT(length == rv);
     return rv;
   }
 
+  // Implementation of writing to a memory buffer.
   static int WriteToMemory(const char* msg, int length) {
-    ASSERT(output_.buffer != NULL);
-    ASSERT(output_buffer_write_pos_ >= output_.buffer);
+    ASSERT(output_buffer_ != NULL);
+    ASSERT(output_buffer_write_pos_ >= output_buffer_);
     if (output_buffer_write_pos_ + length
-        <= output_.buffer + kOutputBufferSize) {
+        <= output_buffer_ + kOutputBufferSize) {
       memcpy(output_buffer_write_pos_, msg, length);
       output_buffer_write_pos_ += length;
       return length;
@@ -334,14 +341,14 @@ class Log : public AllStatic {
     }
   }
 
-  // When logging is active, output_ refers the file or memory buffer
-  // events are written to.
-  // mutex_ should be acquired before using output_.
-  union Output {
-    FILE* handle;
-    char* buffer;
-  };
-  static Output output_;
+  // When logging is active, either output_handle_ or output_buffer_ is used
+  // to store a pointer to log destination. If logging was opened via OpenStdout
+  // or OpenFile, then output_handle_ is used. If logging was opened
+  // via OpenMemoryBuffer, then output_buffer_ is used.
+  // mutex_ should be acquired before using output_handle_ or output_buffer_.
+  static FILE* output_handle_;
+
+  static char* output_buffer_;
 
   // mutex_ is a Mutex used for enforcing exclusive
   // access to the formatting buffer and the log file or log memory buffer.
@@ -365,7 +372,8 @@ class Log : public AllStatic {
 
 
 Log::WritePtr Log::Write = NULL;
-Log::Output Log::output_ = {NULL};
+FILE* Log::output_handle_ = NULL;
+char* Log::output_buffer_ = NULL;
 Mutex* Log::mutex_ = NULL;
 char* Log::output_buffer_write_pos_ = NULL;
 char* Log::message_buffer_ = NULL;
@@ -378,25 +386,25 @@ void Log::Init() {
 
 
 void Log::OpenStdout() {
-  ASSERT(output_.handle == NULL);
-  output_.handle = stdout;
+  ASSERT(!is_enabled());
+  output_handle_ = stdout;
   Write = WriteToFile;
   Init();
 }
 
 
 void Log::OpenFile(const char* name) {
-  ASSERT(output_.handle == NULL);
-  output_.handle = OS::FOpen(name, OS::LogFileOpenMode);
+  ASSERT(!is_enabled());
+  output_handle_ = OS::FOpen(name, OS::LogFileOpenMode);
   Write = WriteToFile;
   Init();
 }
 
 
 void Log::OpenMemoryBuffer() {
-  ASSERT(output_.buffer == NULL);
-  output_.buffer = NewArray<char>(kOutputBufferSize);
-  output_buffer_write_pos_ = output_.buffer;
+  ASSERT(!is_enabled());
+  output_buffer_ = NewArray<char>(kOutputBufferSize);
+  output_buffer_write_pos_ = output_buffer_;
   Write = WriteToMemory;
   Init();
 }
@@ -404,11 +412,11 @@ void Log::OpenMemoryBuffer() {
 
 void Log::Close() {
   if (Write == WriteToFile) {
-    fclose(output_.handle);
-    output_.handle = NULL;
+    fclose(output_handle_);
+    output_handle_ = NULL;
   } else if (Write == WriteToMemory) {
-    DeleteArray(output_.buffer);
-    output_.buffer = NULL;
+    DeleteArray(output_buffer_);
+    output_buffer_ = NULL;
   } else {
     ASSERT(Write == NULL);
   }
@@ -424,15 +432,15 @@ void Log::Close() {
 
 int Log::GetLogLines(int from_pos, char* dest_buf, int max_size) {
   if (Write != WriteToMemory) return 0;
-  ASSERT(output_.buffer != NULL);
-  ASSERT(output_buffer_write_pos_ >= output_.buffer);
+  ASSERT(output_buffer_ != NULL);
+  ASSERT(output_buffer_write_pos_ >= output_buffer_);
   ASSERT(from_pos >= 0);
   ASSERT(max_size >= 0);
   int actual_size = max_size;
-  char* buffer_read_pos = output_.buffer + from_pos;
+  char* buffer_read_pos = output_buffer_ + from_pos;
   ScopedLock sl(mutex_);
   if (actual_size == 0
-      || output_buffer_write_pos_ == output_.buffer
+      || output_buffer_write_pos_ == output_buffer_
       || buffer_read_pos >= output_buffer_write_pos_) {
     // No data requested or can be returned.
     return 0;
@@ -584,7 +592,7 @@ VMState Logger::bottom_state_(EXTERNAL);
 SlidingStateWindow* Logger::sliding_state_window_ = NULL;
 
 
-bool Logger::is_enabled() {
+bool Logger::IsEnabled() {
   return Log::is_enabled();
 }