Make the net logging code more compact.

net/base/net_log_logger.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/base/net_log_logger.h"
 
 #include <stdio.h>
 
 #include "base/json/json_writer.h"
 #include "base/logging.h"
@@ skipping 59 matching lines @@
 base::DictionaryValue* NetLogLogger::GetConstants() {
   base::DictionaryValue* constants_dict = new base::DictionaryValue();
 
   // Version of the file format.
   constants_dict->SetInteger("logFormatVersion", kLogFormatVersion);
 
   // Add a dictionary with information on the relationship between event type
   // enums and their symbolic names.
   constants_dict->Set("logEventTypes", net::NetLog::GetEventTypesAsValue());
 
+  struct StringToConstantTable {
+    const char* name;
+    const int constant;
+    };

[mmenke, 2014/09/15 14:06:08]

nit:  -2 indent, and blank line after struct definition.

   // Add a dictionary with information about the relationship between CertStatus
   // flags and their symbolic names.
   {
     base::DictionaryValue* dict = new base::DictionaryValue();
 
-#define CERT_STATUS_FLAG(label, value) dict->SetInteger(#label, value);
+#define CERT_STATUS_FLAG(label, value) \
+    { #label, (value) },

[mmenke, 2014/09/15 14:06:08]

Can these defines all fit on one row?  (Except perhaps the LOAD_STATE one)

[mmenke, 2014/09/15 14:06:08]

Any reason value is in parentheses here, but not elsewhere?

[Daniel Bratell, 2014/09/15 16:11:51]

I had a compiler error I thought was because of the lack of () but it was
because of the semi colons and then I forgot about it. I will remove them again.

+    static const StringToConstantTable certStatusFlags[] = {

[mmenke, 2014/09/15 14:06:08]

constants should use kConstantNamingScheme

[mmenke, 2014/09/15 14:06:08]

Don't use static:  It makes this code potentially non-thread-safe, I believe.

 #include "net/cert/cert_status_flags_list.h"
+    };
 #undef CERT_STATUS_FLAG

[mmenke, 2014/09/15 14:06:08]

optional:  I think these are little more readable if you put the #ifdef/#undef
immediately abody/below the include statement - that way, the array definition
stands out a bit more, and I think it makes the indentation a bit less
confusing.

[Daniel Bratell, 2014/09/15 16:11:51]

Good suggestion! It did make it more readable!

+    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(certStatusFlags); i++)
+      dict->SetInteger(certStatusFlags[i].name, certStatusFlags[i].constant);
 
     constants_dict->Set("certStatusFlag", dict);
   }
 
   // Add a dictionary with information about the relationship between load flag
   // enums and their symbolic names.
   {
     base::DictionaryValue* dict = new base::DictionaryValue();
 
 #define LOAD_FLAG(label, value) \
-    dict->SetInteger(# label, static_cast<int>(value));
+    { #label, value },
+    static const StringToConstantTable loadFlags[] = {
 #include "net/base/load_flags_list.h"
+    }; // End loadFlags.
 #undef LOAD_FLAG
+    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(loadFlags); i++)

[mmenke, 2014/09/15 14:06:08]

Looks like we aren't including the header for ARRAYSIZE_UNSAFE.  Could you add
base/macros.h to net_log_logger.h (Which should already have it, for
DISALLOW_COPY_AND_ASSIGN, but does not).

+      dict->SetInteger(loadFlags[i].name, loadFlags[i].constant);
 
     constants_dict->Set("loadFlag", dict);
   }
 
   // Add a dictionary with information about the relationship between load state
   // enums and their symbolic names.
   {
     base::DictionaryValue* dict = new base::DictionaryValue();
 
 #define LOAD_STATE(label) \
-    dict->SetInteger(# label, net::LOAD_STATE_ ## label);
+      { # label, net::LOAD_STATE_ ## label },

[mmenke, 2014/09/15 14:06:08]

Believe this should use a 2-space indent, like the others.

[mmenke, 2014/09/15 14:37:41]

Sorry, that should have been 4-space indent.

+    static const StringToConstantTable loadStateTable[] = {
 #include "net/base/load_states_list.h"
+    }; // End loadStateTable.

[mmenke, 2014/09/15 14:06:08]

optional nit:  I don't think this comment adds anything.  If you disagree, feel
free to keep them, but add one space before the comment (Should be two spaces
between code and comments).

 #undef LOAD_STATE
+    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(loadStateTable); i++)
+      dict->SetInteger(loadStateTable[i].name, loadStateTable[i].constant);
 
     constants_dict->Set("loadState", dict);
   }
 
   // Add information on the relationship between net error codes and their
   // symbolic names.
   {
     base::DictionaryValue* dict = new base::DictionaryValue();
 
-#define NET_ERROR(label, value) \
-    dict->SetInteger(ErrorToShortString(value), static_cast<int>(value));
+#define NET_ERROR(label, value) value,
+    static const int errorList[] = {
 #include "net/base/net_error_list.h"
+    }; // end errorList.
 #undef NET_ERROR
+    for (size_t i = 0; i < ARRAYSIZE_UNSAFE(errorList); i++)
+      dict->SetInteger(ErrorToShortString(errorList[i]), errorList[i]);
 
     constants_dict->Set("netError", dict);
   }
 
   // Add information on the relationship between QUIC error codes and their
   // symbolic names.
   {
     base::DictionaryValue* dict = new base::DictionaryValue();
 
     for (net::QuicErrorCode error = net::QUIC_NO_ERROR;
@@ skipping 90 matching lines @@
   }
 
   // "clientInfo" key is required for some NetLogLogger log readers.
   // Provide a default empty value for compatibility.
   constants_dict->Set("clientInfo", new base::DictionaryValue());
 
   return constants_dict;
 }
 
 }  // namespace net