Define a LoggingSettings struct to use for InitLogging()

base/logging.h

 // Copyright (c) 2012 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.
 
 #ifndef BASE_LOGGING_H_
 #define BASE_LOGGING_H_
 
 #include <cassert>
 #include <string>
 #include <cstring>
@@ skipping 128 matching lines @@
 //
 // Note the special severity of ERROR_REPORT only available/relevant in normal
 // mode, which displays error dialog without terminating the program. There is
 // no error dialog for severity ERROR or below in normal mode.
 //
 // There is also the special severity of DFATAL, which logs FATAL in
 // debug mode, ERROR in normal mode.
 
 namespace logging {
 
-// Where to record logging output? A flat file and/or system debug log via
-// OutputDebugString. Defaults on Windows to LOG_ONLY_TO_FILE, and on
-// POSIX to LOG_ONLY_TO_SYSTEM_DEBUG_LOG (aka stderr).
-enum LoggingDestination { LOG_NONE,
-                          LOG_ONLY_TO_FILE,
-                          LOG_ONLY_TO_SYSTEM_DEBUG_LOG,
-                          LOG_TO_BOTH_FILE_AND_SYSTEM_DEBUG_LOG };
+// TODO(avi): do we want to do a unification of character types here?
+#if defined(OS_WIN)
+typedef wchar_t PathChar;
+#else
+typedef char PathChar;
+#endif
+
+// Where to record logging output? A flat file and/or system debug log
+// via OutputDebugString.
+enum LoggingDestination {
+  LOG_NONE                = 0,
+  LOG_TO_FILE             = 1 << 0,
+  LOG_TO_SYSTEM_DEBUG_LOG = 1 << 1,
+
+  LOG_TO_ALL = LOG_TO_FILE | LOG_TO_SYSTEM_DEBUG_LOG,
+
+  // On Windows, use a file next to the exe; on POSIX platforms, where
+  // it may not even be possible to locate the executable on disk, use
+  // stderr.
+#if defined(OS_WIN)
+  LOG_DEFAULT = LOG_TO_FILE,
+#elif defined(OS_POSIX)
+  LOG_DEFAULT = LOG_TO_SYSTEM_DEBUG_LOG,
+#endif
+};
 
 // Indicates that the log file should be locked when being written to.
-// Often, there is no locking, which is fine for a single threaded program.
-// If logging is being done from multiple threads or there can be more than
-// one process doing the logging, the file should be locked during writes to
-// make each log outut atomic. Other writers will block.
+// Unless there is only one single-threaded process that is logging to
+// the log file, the file should be locked during writes to make each
+// log outut atomic. Other writers will block.
 //
 // All processes writing to the log file must have their locking set for it to
-// work properly. Defaults to DONT_LOCK_LOG_FILE.
+// work properly. Defaults to LOCK_LOG_FILE.
 enum LogLockingState { LOCK_LOG_FILE, DONT_LOCK_LOG_FILE };
 
 // On startup, should we delete or append to an existing log file (if any)?
 // Defaults to APPEND_TO_OLD_LOG_FILE.
 enum OldFileDeletionState { DELETE_OLD_LOG_FILE, APPEND_TO_OLD_LOG_FILE };
 
 enum DcheckState {
   DISABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS,

[brettw, 2013/06/19 18:10:13]

I actually think this would be a good opportunity to delete this enum. The long
names were to make the calls to InitLogging clear (rather than seeing a call
with a bunch of random bools to it).

But with your struct we can have a nicely named bool "enable_..." and just have
people set it to true or false. Maybe log locking state too
("settings.lock_log_file = true;").

[akalin, 2013/06/19 19:59:18]

I tried this, but I ultimately decided it wasn't a good idea.

Even though InitLogging itself takes a struct, these enums are passed around to
other functions/classes that don't take the struct, and so those would turn into
functions that would take bools, which isn't great.

For LogLockingState in particular, LoggingLock::Init() takes it as a parameter.

For DcheckState in particular, I couldn't think of a better name than 'bool
enable_dcheck_for_non_official_release_builds;', so it wouldn't save much space
anyway. (enable_dcheck by itself could be misleading.)

OldFileDeletionState is used only in LoggingSettings, but I think it's still
useful to have the enum as it's not strictly an on/off thing (it's not
immediately obvious that not deleting == appending).

   ENABLE_DCHECK_FOR_NON_OFFICIAL_RELEASE_BUILDS
 };
 
-// TODO(avi): do we want to do a unification of character types here?
-#if defined(OS_WIN)
-typedef wchar_t PathChar;
-#else
-typedef char PathChar;
-#endif
+struct BASE_EXPORT LoggingSettings {

[brettw, 2013/06/19 18:10:13]

I think the settings here should all be explicit about what the defaults are.
One of the things about this patch is that you don't set things when the default
is OK, which means that we should guarantee in the header how such things are
done.

[akalin, 2013/06/19 19:59:18]

Done. Some of the info is in the comments for the enums, but it's probably good
to put them here anyway.

+  LoggingSettings();
+
+  LoggingDestination logging_dest;
+
+  // The three settings below have an effect only when LOG_TO_FILE is
+  // set in |logging_dest|.
+  const PathChar* log_file;
+  LogLockingState lock_log;
+  OldFileDeletionState delete_old;
+
+  DcheckState dcheck_state;
+};
 
 // Define different names for the BaseInitLoggingImpl() function depending on
 // whether NDEBUG is defined or not so that we'll fail to link if someone tries
 // to compile logging.cc with NDEBUG but includes logging.h without defining it,
 // or vice versa.
 #if NDEBUG
 #define BaseInitLoggingImpl BaseInitLoggingImpl_built_with_NDEBUG
 #else
 #define BaseInitLoggingImpl BaseInitLoggingImpl_built_without_NDEBUG
 #endif
 
 // Implementation of the InitLogging() method declared below.  We use a
 // more-specific name so we can #define it above without affecting other code
 // that has named stuff "InitLogging".
-BASE_EXPORT bool BaseInitLoggingImpl(const PathChar* log_file,
-                                     LoggingDestination logging_dest,
-                                     LogLockingState lock_log,
-                                     OldFileDeletionState delete_old,
-                                     DcheckState dcheck_state);
+BASE_EXPORT bool BaseInitLoggingImpl(const LoggingSettings& settings);
 
 // Sets the log file name and other global logging state. Calling this function
 // is recommended, and is normally done at the beginning of application init.
 // If you don't call it, all the flags will be initialized to their default
 // values, and there is a race condition that may leak a critical section
 // object if two threads try to do the first log at the same time.
 // See the definition of the enums above for descriptions and default values.
 //
 // The default log file is initialized to "debug.log" in the application
 // directory. You probably don't want this, especially since the program
 // directory may not be writable on an enduser's system.
 //
 // This function may be called a second time to re-direct logging (e.g after
 // loging in to a user partition), however it should never be called more than
 // twice.
-inline bool InitLogging(const PathChar* log_file,
-                        LoggingDestination logging_dest,
-                        LogLockingState lock_log,
-                        OldFileDeletionState delete_old,
-                        DcheckState dcheck_state) {
-  return BaseInitLoggingImpl(log_file, logging_dest, lock_log,
-                             delete_old, dcheck_state);
+inline bool InitLogging(const LoggingSettings& settings) {
+  return BaseInitLoggingImpl(settings);
 }
 
 // Sets the log level. Anything at or above this level will be written to the
 // log file/displayed to the user (if applicable). Anything below this level
 // will be silently ignored. The log level defaults to 0 (everything is logged
 // up to level INFO) if this function is not called.
 // Note that log messages for VLOG(x) are logged at level -x, so setting
 // the min log level to negative values enables verbose logging.
 BASE_EXPORT void SetMinLogLevel(int level);
 
@@ skipping 750 matching lines @@
 #elif NOTIMPLEMENTED_POLICY == 5
 #define NOTIMPLEMENTED() do {\
   static bool logged_once = false;\
   LOG_IF(ERROR, !logged_once) << NOTIMPLEMENTED_MSG;\
   logged_once = true;\
 } while(0);\
 EAT_STREAM_PARAMETERS
 #endif
 
 #endif  // BASE_LOGGING_H_