From f55f4fcf05a53fdf618b4c69ddcf4c43b14e84c2 Mon Sep 17 00:00:00 2001
From: Jim Posen <jimpo@coinbase.com>
Date: Wed, 11 Apr 2018 10:03:21 -0700
Subject: [PATCH 1/6] util: Establish global logger object.

The object encapsulates logging configuration, and in a later commit,
set up routines will also be moved into the class.
---
 src/bench/bench_bitcoin.cpp |  2 +-
 src/init.cpp                | 15 ++++++------
 src/logging.cpp             | 37 +++++++++++++++--------------
 src/logging.h               | 47 +++++++++++++++++++++++++++----------
 src/test/test_bitcoin.cpp   |  2 +-
 src/util.h                  |  2 +-
 6 files changed, 66 insertions(+), 39 deletions(-)

diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp
index 1d8788352..f08c099c1 100644
--- a/src/bench/bench_bitcoin.cpp
+++ b/src/bench/bench_bitcoin.cpp
@@ -46,7 +46,7 @@ main(int argc, char** argv)
     RandomInit();
     ECC_Start();
     SetupEnvironment();
-    fPrintToDebugLog = false; // don't want to write to debug.log file
+    g_logger->fPrintToDebugLog = false; // don't want to write to debug.log file
 
     int64_t evaluations = gArgs.GetArg("-evals", DEFAULT_BENCH_EVALUATIONS);
     std::string regex_filter = gArgs.GetArg("-filter", DEFAULT_BENCH_FILTER);
diff --git a/src/init.cpp b/src/init.cpp
index 99dab605a..814fd3944 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -305,7 +305,7 @@ static void HandleSIGTERM(int)
 
 static void HandleSIGHUP(int)
 {
-    fReopenDebugLog = true;
+    g_logger->fReopenDebugLog = true;
 }
 
 #ifndef WIN32
@@ -831,10 +831,11 @@ void InitLogging()
     // debug.log.
     LogPrintf("\n\n\n\n\n");
 
-    fPrintToConsole = gArgs.GetBoolArg("-printtoconsole", !gArgs.GetBoolArg("-daemon", false));
-    fPrintToDebugLog = !gArgs.IsArgNegated("-debuglogfile");
-    fLogTimestamps = gArgs.GetBoolArg("-logtimestamps", DEFAULT_LOGTIMESTAMPS);
-    fLogTimeMicros = gArgs.GetBoolArg("-logtimemicros", DEFAULT_LOGTIMEMICROS);
+    g_logger->fPrintToConsole = gArgs.GetBoolArg("-printtoconsole", !gArgs.GetBoolArg("-daemon", false));
+    g_logger->fPrintToDebugLog = !gArgs.IsArgNegated("-debuglogfile");
+    g_logger->fLogTimestamps = gArgs.GetBoolArg("-logtimestamps", DEFAULT_LOGTIMESTAMPS);
+    g_logger->fLogTimeMicros = gArgs.GetBoolArg("-logtimemicros", DEFAULT_LOGTIMEMICROS);
+
     fLogIPs = gArgs.GetBoolArg("-logips", DEFAULT_LOGIPS);
 
     std::string version_string = FormatFullVersion();
@@ -1230,7 +1231,7 @@ bool AppInitMain()
 #ifndef WIN32
     CreatePidFile(GetPidFile(), getpid());
 #endif
-    if (fPrintToDebugLog) {
+    if (g_logger->fPrintToDebugLog) {
         if (gArgs.GetBoolArg("-shrinkdebugfile", logCategories == BCLog::NONE)) {
             // Do this first since it both loads a bunch of debug.log into memory,
             // and because this needs to happen before any other debug.log printing
@@ -1241,7 +1242,7 @@ bool AppInitMain()
         }
     }
 
-    if (!fLogTimestamps)
+    if (!g_logger->fLogTimestamps)
         LogPrintf("Startup time: %s\n", FormatISO8601DateTime(GetTime()));
     LogPrintf("Default data directory %s\n", GetDefaultDataDir().string());
     LogPrintf("Using data directory %s\n", GetDataDir().string());
diff --git a/src/logging.cpp b/src/logging.cpp
index e48158232..de222d946 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -12,13 +12,22 @@
 
 const char * const DEFAULT_DEBUGLOGFILE = "debug.log";
 
-bool fPrintToConsole = false;
-bool fPrintToDebugLog = true;
+/**
+ * NOTE: the logger instances is leaked on exit. This is ugly, but will be
+ * cleaned up by the OS/libc. Defining a logger as a global object doesn't work
+ * since the order of destruction of static/global objects is undefined.
+ * Consider if the logger gets destroyed, and then some later destructor calls
+ * LogPrintf, maybe indirectly, and you get a core dump at shutdown trying to
+ * access the logger. When the shutdown sequence is fully audited and tested,
+ * explicit destruction of these objects can be implemented by changing this
+ * from a raw pointer to a std::unique_ptr.
+ *
+ * This method of initialization was originally introduced in
+ * ee3374234c60aba2cc4c5cd5cac1c0aefc2d817c.
+ */
+BCLog::Logger* const g_logger = new BCLog::Logger();
 
-bool fLogTimestamps = DEFAULT_LOGTIMESTAMPS;
-bool fLogTimeMicros = DEFAULT_LOGTIMEMICROS;
 bool fLogIPs = DEFAULT_LOGIPS;
-std::atomic<bool> fReopenDebugLog(false);
 
 /** Log categories bitfield. */
 std::atomic<uint32_t> logCategories(0);
@@ -174,19 +183,14 @@ std::vector<CLogCategoryActive> ListActiveLogCategories()
     return ret;
 }
 
-/**
- * fStartedNewLine is a state variable held by the calling context that will
- * suppress printing of the timestamp when multiple calls are made that don't
- * end in a newline. Initialize it to true, and hold it, in the calling context.
- */
-static std::string LogTimestampStr(const std::string &str, std::atomic_bool *fStartedNewLine)
+std::string BCLog::Logger::LogTimestampStr(const std::string &str)
 {
     std::string strStamped;
 
     if (!fLogTimestamps)
         return str;
 
-    if (*fStartedNewLine) {
+    if (fStartedNewLine) {
         int64_t nTimeMicros = GetTimeMicros();
         strStamped = FormatISO8601DateTime(nTimeMicros/1000000);
         if (fLogTimeMicros) {
@@ -202,19 +206,18 @@ static std::string LogTimestampStr(const std::string &str, std::atomic_bool *fSt
         strStamped = str;
 
     if (!str.empty() && str[str.size()-1] == '\n')
-        *fStartedNewLine = true;
+        fStartedNewLine = true;
     else
-        *fStartedNewLine = false;
+        fStartedNewLine = false;
 
     return strStamped;
 }
 
-int LogPrintStr(const std::string &str)
+int BCLog::Logger::LogPrintStr(const std::string &str)
 {
     int ret = 0; // Returns total number of characters written
-    static std::atomic_bool fStartedNewLine(true);
 
-    std::string strTimestamped = LogTimestampStr(str, &fStartedNewLine);
+    std::string strTimestamped = LogTimestampStr(str);
 
     if (fPrintToConsole) {
         // print to console
diff --git a/src/logging.h b/src/logging.h
index 4053f75ac..1ccf9136f 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -19,13 +19,7 @@ static const bool DEFAULT_LOGIPS        = false;
 static const bool DEFAULT_LOGTIMESTAMPS = true;
 extern const char * const DEFAULT_DEBUGLOGFILE;
 
-extern bool fPrintToConsole;
-extern bool fPrintToDebugLog;
-
-extern bool fLogTimestamps;
-extern bool fLogTimeMicros;
 extern bool fLogIPs;
-extern std::atomic<bool> fReopenDebugLog;
 
 extern std::atomic<uint32_t> logCategories;
 
@@ -61,7 +55,39 @@ namespace BCLog {
         LEVELDB     = (1 << 20),
         ALL         = ~(uint32_t)0,
     };
-}
+
+    class Logger
+    {
+    private:
+        /**
+         * fStartedNewLine is a state variable that will suppress printing of
+         * the timestamp when multiple calls are made that don't end in a
+         * newline.
+         */
+        std::atomic_bool fStartedNewLine{true};
+
+        std::string LogTimestampStr(const std::string& str);
+
+    public:
+        bool fPrintToConsole = false;
+        bool fPrintToDebugLog = true;
+
+        bool fLogTimestamps = DEFAULT_LOGTIMESTAMPS;
+        bool fLogTimeMicros = DEFAULT_LOGTIMEMICROS;
+
+        std::atomic<bool> fReopenDebugLog{false};
+
+        /** Send a string to the log output */
+        int LogPrintStr(const std::string &str);
+
+        /** Returns whether logs will be written to any output */
+        bool Enabled() const { return fPrintToConsole || fPrintToDebugLog; }
+    };
+
+} // namespace BCLog
+
+extern BCLog::Logger* const g_logger;
+
 /** Return true if log accepts specified category */
 static inline bool LogAcceptCategory(uint32_t category)
 {
@@ -77,9 +103,6 @@ std::vector<CLogCategoryActive> ListActiveLogCategories();
 /** Return true if str parses as a log category and set the flags in f */
 bool GetLogCategory(uint32_t *f, const std::string *str);
 
-/** Send a string to the log output */
-int LogPrintStr(const std::string &str);
-
 /** Get format string from VA_ARGS for error reporting */
 template<typename... Args> std::string FormatStringFromLogArgs(const char *fmt, const Args&... args) { return fmt; }
 
@@ -99,7 +122,7 @@ template<typename T, typename... Args> static inline void MarkUsed(const T& t, c
 #define LogPrint(category, ...) do { MarkUsed(__VA_ARGS__); } while(0)
 #else
 #define LogPrintf(...) do { \
-    if (fPrintToConsole || fPrintToDebugLog) { \
+    if (g_logger->Enabled()) { \
         std::string _log_msg_; /* Unlikely name to avoid shadowing variables */ \
         try { \
             _log_msg_ = tfm::format(__VA_ARGS__); \
@@ -107,7 +130,7 @@ template<typename T, typename... Args> static inline void MarkUsed(const T& t, c
             /* Original format string will have newline so don't add one here */ \
             _log_msg_ = "Error \"" + std::string(fmterr.what()) + "\" while formatting log message: " + FormatStringFromLogArgs(__VA_ARGS__); \
         } \
-        LogPrintStr(_log_msg_); \
+        g_logger->LogPrintStr(_log_msg_); \
     } \
 } while(0)
 
diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp
index e9873f452..fa59a9ce3 100644
--- a/src/test/test_bitcoin.cpp
+++ b/src/test/test_bitcoin.cpp
@@ -47,7 +47,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName)
         SetupNetworking();
         InitSignatureCache();
         InitScriptExecutionCache();
-        fPrintToDebugLog = false; // don't want to write to debug.log file
+        g_logger->fPrintToDebugLog = false; // don't want to write to debug.log file
         fCheckBlockIndex = true;
         SelectParams(chainName);
         noui_connect();
diff --git a/src/util.h b/src/util.h
index ce94f396a..2da802328 100644
--- a/src/util.h
+++ b/src/util.h
@@ -66,7 +66,7 @@ bool SetupNetworking();
 template<typename... Args>
 bool error(const char* fmt, const Args&... args)
 {
-    LogPrintStr("ERROR: " + tfm::format(fmt, args...) + "\n");
+    LogPrintf("ERROR: %s\n", tfm::format(fmt, args...));
     return false;
 }
 

From 6a6d764ca5616e5d1f1848b0010613c49bd38e61 Mon Sep 17 00:00:00 2001
From: Jim Posen <jimpo@coinbase.com>
Date: Wed, 11 Apr 2018 11:12:51 -0700
Subject: [PATCH 2/6] util: Move debug file management functions into Logger.

---
 src/init.cpp    |  7 +++---
 src/logging.cpp | 59 ++++++++-----------------------------------------
 src/logging.h   | 14 ++++++++----
 3 files changed, 23 insertions(+), 57 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index 814fd3944..c14597d51 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1235,10 +1235,11 @@ bool AppInitMain()
         if (gArgs.GetBoolArg("-shrinkdebugfile", logCategories == BCLog::NONE)) {
             // Do this first since it both loads a bunch of debug.log into memory,
             // and because this needs to happen before any other debug.log printing
-            ShrinkDebugFile();
+            g_logger->ShrinkDebugFile();
         }
-        if (!OpenDebugLog()) {
-            return InitError(strprintf("Could not open debug log file %s", GetDebugLogPath().string()));
+        if (!g_logger->OpenDebugLog()) {
+            return InitError(strprintf("Could not open debug log file %s",
+                                       g_logger->GetDebugLogPath().string()));
         }
     }
 
diff --git a/src/logging.cpp b/src/logging.cpp
index de222d946..ed225a6a6 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -7,9 +7,6 @@
 #include <util.h>
 #include <utilstrencodings.h>
 
-#include <list>
-#include <mutex>
-
 const char * const DEFAULT_DEBUGLOGFILE = "debug.log";
 
 /**
@@ -31,57 +28,23 @@ bool fLogIPs = DEFAULT_LOGIPS;
 
 /** Log categories bitfield. */
 std::atomic<uint32_t> logCategories(0);
-/**
- * LogPrintf() has been broken a couple of times now
- * by well-meaning people adding mutexes in the most straightforward way.
- * It breaks because it may be called by global destructors during shutdown.
- * Since the order of destruction of static/global objects is undefined,
- * defining a mutex as a global object doesn't work (the mutex gets
- * destroyed, and then some later destructor calls OutputDebugStringF,
- * maybe indirectly, and you get a core dump at shutdown trying to lock
- * the mutex).
- */
-
-static std::once_flag debugPrintInitFlag;
-
-/**
- * We use std::call_once() to make sure mutexDebugLog and
- * vMsgsBeforeOpenLog are initialized in a thread-safe manner.
- *
- * NOTE: fileout, mutexDebugLog and sometimes vMsgsBeforeOpenLog
- * are leaked on exit. This is ugly, but will be cleaned up by
- * the OS/libc. When the shutdown sequence is fully audited and
- * tested, explicit destruction of these objects can be implemented.
- */
-static FILE* fileout = nullptr;
-static std::mutex* mutexDebugLog = nullptr;
-static std::list<std::string>* vMsgsBeforeOpenLog;
 
 static int FileWriteStr(const std::string &str, FILE *fp)
 {
     return fwrite(str.data(), 1, str.size(), fp);
 }
 
-static void DebugPrintInit()
-{
-    assert(mutexDebugLog == nullptr);
-    mutexDebugLog = new std::mutex();
-    vMsgsBeforeOpenLog = new std::list<std::string>;
-}
-
-fs::path GetDebugLogPath()
+fs::path BCLog::Logger::GetDebugLogPath() const
 {
     fs::path logfile(gArgs.GetArg("-debuglogfile", DEFAULT_DEBUGLOGFILE));
     return AbsPathForConfigVal(logfile);
 }
 
-bool OpenDebugLog()
+bool BCLog::Logger::OpenDebugLog()
 {
-    std::call_once(debugPrintInitFlag, &DebugPrintInit);
-    std::lock_guard<std::mutex> scoped_lock(*mutexDebugLog);
+    std::lock_guard<std::mutex> scoped_lock(mutexDebugLog);
 
     assert(fileout == nullptr);
-    assert(vMsgsBeforeOpenLog);
     fs::path pathDebug = GetDebugLogPath();
 
     fileout = fsbridge::fopen(pathDebug, "a");
@@ -91,13 +54,11 @@ bool OpenDebugLog()
 
     setbuf(fileout, nullptr); // unbuffered
     // dump buffered messages from before we opened the log
-    while (!vMsgsBeforeOpenLog->empty()) {
-        FileWriteStr(vMsgsBeforeOpenLog->front(), fileout);
-        vMsgsBeforeOpenLog->pop_front();
+    while (!vMsgsBeforeOpenLog.empty()) {
+        FileWriteStr(vMsgsBeforeOpenLog.front(), fileout);
+        vMsgsBeforeOpenLog.pop_front();
     }
 
-    delete vMsgsBeforeOpenLog;
-    vMsgsBeforeOpenLog = nullptr;
     return true;
 }
 
@@ -225,14 +186,12 @@ int BCLog::Logger::LogPrintStr(const std::string &str)
         fflush(stdout);
     }
     if (fPrintToDebugLog) {
-        std::call_once(debugPrintInitFlag, &DebugPrintInit);
-        std::lock_guard<std::mutex> scoped_lock(*mutexDebugLog);
+        std::lock_guard<std::mutex> scoped_lock(mutexDebugLog);
 
         // buffer if we haven't opened the log yet
         if (fileout == nullptr) {
-            assert(vMsgsBeforeOpenLog);
             ret = strTimestamped.length();
-            vMsgsBeforeOpenLog->push_back(strTimestamped);
+            vMsgsBeforeOpenLog.push_back(strTimestamped);
         }
         else
         {
@@ -250,7 +209,7 @@ int BCLog::Logger::LogPrintStr(const std::string &str)
     return ret;
 }
 
-void ShrinkDebugFile()
+void BCLog::Logger::ShrinkDebugFile()
 {
     // Amount of debug.log to save at end when shrinking (must fit in memory)
     constexpr size_t RECENT_DEBUG_HISTORY_SIZE = 10 * 1000000;
diff --git a/src/logging.h b/src/logging.h
index 1ccf9136f..c27a71168 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -11,6 +11,8 @@
 
 #include <atomic>
 #include <cstdint>
+#include <list>
+#include <mutex>
 #include <string>
 #include <vector>
 
@@ -59,6 +61,10 @@ namespace BCLog {
     class Logger
     {
     private:
+        FILE* fileout = nullptr;
+        std::mutex mutexDebugLog;
+        std::list<std::string> vMsgsBeforeOpenLog;
+
         /**
          * fStartedNewLine is a state variable that will suppress printing of
          * the timestamp when multiple calls are made that don't end in a
@@ -82,6 +88,10 @@ namespace BCLog {
 
         /** Returns whether logs will be written to any output */
         bool Enabled() const { return fPrintToConsole || fPrintToDebugLog; }
+
+        fs::path GetDebugLogPath() const;
+        bool OpenDebugLog();
+        void ShrinkDebugFile();
     };
 
 } // namespace BCLog
@@ -141,8 +151,4 @@ template<typename T, typename... Args> static inline void MarkUsed(const T& t, c
 } while(0)
 #endif
 
-fs::path GetDebugLogPath();
-bool OpenDebugLog();
-void ShrinkDebugFile();
-
 #endif // BITCOIN_LOGGING_H

From 3316a9ebb66171937efddb213daed64fe51c4082 Mon Sep 17 00:00:00 2001
From: Jim Posen <jimpo@coinbase.com>
Date: Wed, 11 Apr 2018 13:02:01 -0700
Subject: [PATCH 3/6] util: Encapsulate logCategories within BCLog::Logger.

---
 src/httpserver.cpp      |  4 ++--
 src/httpserver.h        |  2 +-
 src/init.cpp            |  6 +++---
 src/interfaces/node.cpp |  2 +-
 src/logging.cpp         | 23 ++++++++++++++++++++---
 src/logging.h           | 16 ++++++++++++----
 src/rpc/misc.cpp        | 30 ++++++++++++++++--------------
 7 files changed, 55 insertions(+), 28 deletions(-)

diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index b8b338dfb..bd08b04c0 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -364,8 +364,8 @@ bool InitHTTPServer()
     // Update libevent's log handling. Returns false if our version of
     // libevent doesn't support debug logging, in which case we should
     // clear the BCLog::LIBEVENT flag.
-    if (!UpdateHTTPServerLogging(logCategories & BCLog::LIBEVENT)) {
-        logCategories &= ~BCLog::LIBEVENT;
+    if (!UpdateHTTPServerLogging(g_logger->WillLogCategory(BCLog::LIBEVENT))) {
+        g_logger->DisableCategory(BCLog::LIBEVENT);
     }
 
 #ifdef WIN32
diff --git a/src/httpserver.h b/src/httpserver.h
index f17be6596..8132c887b 100644
--- a/src/httpserver.h
+++ b/src/httpserver.h
@@ -32,7 +32,7 @@ void InterruptHTTPServer();
 /** Stop HTTP server */
 void StopHTTPServer();
 
-/** Change logging level for libevent. Removes BCLog::LIBEVENT from logCategories if
+/** Change logging level for libevent. Removes BCLog::LIBEVENT from log categories if
  * libevent doesn't support debug logging.*/
 bool UpdateHTTPServerLogging(bool enable);
 
diff --git a/src/init.cpp b/src/init.cpp
index c14597d51..ccaa09a85 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -968,7 +968,7 @@ bool AppInitParameterInteraction()
                     InitWarning(strprintf(_("Unsupported logging category %s=%s."), "-debug", cat));
                     continue;
                 }
-                logCategories |= flag;
+                g_logger->EnableCategory(static_cast<BCLog::LogFlags>(flag));
             }
         }
     }
@@ -980,7 +980,7 @@ bool AppInitParameterInteraction()
             InitWarning(strprintf(_("Unsupported logging category %s=%s."), "-debugexclude", cat));
             continue;
         }
-        logCategories &= ~flag;
+        g_logger->DisableCategory(static_cast<BCLog::LogFlags>(flag));
     }
 
     // Check for -debugnet
@@ -1232,7 +1232,7 @@ bool AppInitMain()
     CreatePidFile(GetPidFile(), getpid());
 #endif
     if (g_logger->fPrintToDebugLog) {
-        if (gArgs.GetBoolArg("-shrinkdebugfile", logCategories == BCLog::NONE)) {
+        if (gArgs.GetBoolArg("-shrinkdebugfile", g_logger->DefaultShrinkDebugFile())) {
             // Do this first since it both loads a bunch of debug.log into memory,
             // and because this needs to happen before any other debug.log printing
             g_logger->ShrinkDebugFile();
diff --git a/src/interfaces/node.cpp b/src/interfaces/node.cpp
index 55786c807..53d2359ca 100644
--- a/src/interfaces/node.cpp
+++ b/src/interfaces/node.cpp
@@ -60,7 +60,7 @@ class NodeImpl : public Node
     void initLogging() override { InitLogging(); }
     void initParameterInteraction() override { InitParameterInteraction(); }
     std::string getWarnings(const std::string& type) override { return GetWarnings(type); }
-    uint32_t getLogCategories() override { return ::logCategories; }
+    uint32_t getLogCategories() override { return g_logger->GetCategoryMask(); }
     bool baseInitialize() override
     {
         return AppInitBasicSetup() && AppInitParameterInteraction() && AppInitSanityChecks() &&
diff --git a/src/logging.cpp b/src/logging.cpp
index ed225a6a6..7604c0fd9 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -26,9 +26,6 @@ BCLog::Logger* const g_logger = new BCLog::Logger();
 
 bool fLogIPs = DEFAULT_LOGIPS;
 
-/** Log categories bitfield. */
-std::atomic<uint32_t> logCategories(0);
-
 static int FileWriteStr(const std::string &str, FILE *fp)
 {
     return fwrite(str.data(), 1, str.size(), fp);
@@ -62,6 +59,26 @@ bool BCLog::Logger::OpenDebugLog()
     return true;
 }
 
+void BCLog::Logger::EnableCategory(BCLog::LogFlags flag)
+{
+    logCategories |= flag;
+}
+
+void BCLog::Logger::DisableCategory(BCLog::LogFlags flag)
+{
+    logCategories &= ~flag;
+}
+
+bool BCLog::Logger::WillLogCategory(BCLog::LogFlags category) const
+{
+    return (logCategories.load(std::memory_order_relaxed) & category) != 0;
+}
+
+bool BCLog::Logger::DefaultShrinkDebugFile() const
+{
+    return logCategories == BCLog::NONE;
+}
+
 struct CLogCategoryDesc
 {
     uint32_t flag;
diff --git a/src/logging.h b/src/logging.h
index c27a71168..7a1c25233 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -23,8 +23,6 @@ extern const char * const DEFAULT_DEBUGLOGFILE;
 
 extern bool fLogIPs;
 
-extern std::atomic<uint32_t> logCategories;
-
 struct CLogCategoryActive
 {
     std::string category;
@@ -72,6 +70,9 @@ namespace BCLog {
          */
         std::atomic_bool fStartedNewLine{true};
 
+        /** Log categories bitfield. */
+        std::atomic<uint32_t> logCategories{0};
+
         std::string LogTimestampStr(const std::string& str);
 
     public:
@@ -92,6 +93,13 @@ namespace BCLog {
         fs::path GetDebugLogPath() const;
         bool OpenDebugLog();
         void ShrinkDebugFile();
+
+        uint32_t GetCategoryMask() const { return logCategories.load(); }
+        void EnableCategory(LogFlags flag);
+        void DisableCategory(LogFlags flag);
+        bool WillLogCategory(LogFlags category) const;
+
+        bool DefaultShrinkDebugFile() const;
     };
 
 } // namespace BCLog
@@ -99,9 +107,9 @@ namespace BCLog {
 extern BCLog::Logger* const g_logger;
 
 /** Return true if log accepts specified category */
-static inline bool LogAcceptCategory(uint32_t category)
+static inline bool LogAcceptCategory(BCLog::LogFlags category)
 {
-    return (logCategories.load(std::memory_order_relaxed) & category) != 0;
+    return g_logger->WillLogCategory(category);
 }
 
 /** Returns a string with the log categories. */
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
index 6754407db..26bf21356 100644
--- a/src/rpc/misc.cpp
+++ b/src/rpc/misc.cpp
@@ -346,9 +346,8 @@ UniValue getmemoryinfo(const JSONRPCRequest& request)
     }
 }
 
-uint32_t getCategoryMask(UniValue cats) {
+void EnableOrDisableLogCategories(UniValue cats, bool enable) {
     cats = cats.get_array();
-    uint32_t mask = 0;
     for (unsigned int i = 0; i < cats.size(); ++i) {
         uint32_t flag = 0;
         std::string cat = cats[i].get_str();
@@ -356,11 +355,14 @@ uint32_t getCategoryMask(UniValue cats) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "unknown logging category " + cat);
         }
         if (flag == BCLog::NONE) {
-            return 0;
+            return;
+        }
+        if (enable) {
+            g_logger->EnableCategory(static_cast<BCLog::LogFlags>(flag));
+        } else {
+            g_logger->DisableCategory(static_cast<BCLog::LogFlags>(flag));
         }
-        mask |= flag;
     }
-    return mask;
 }
 
 UniValue logging(const JSONRPCRequest& request)
@@ -399,25 +401,25 @@ UniValue logging(const JSONRPCRequest& request)
         );
     }
 
-    uint32_t originalLogCategories = logCategories;
+    uint32_t original_log_categories = g_logger->GetCategoryMask();
     if (request.params[0].isArray()) {
-        logCategories |= getCategoryMask(request.params[0]);
+        EnableOrDisableLogCategories(request.params[0], true);
     }
-
     if (request.params[1].isArray()) {
-        logCategories &= ~getCategoryMask(request.params[1]);
+        EnableOrDisableLogCategories(request.params[1], false);
     }
+    uint32_t updated_log_categories = g_logger->GetCategoryMask();
+    uint32_t changed_log_categories = original_log_categories ^ updated_log_categories;
 
     // Update libevent logging if BCLog::LIBEVENT has changed.
     // If the library version doesn't allow it, UpdateHTTPServerLogging() returns false,
     // in which case we should clear the BCLog::LIBEVENT flag.
     // Throw an error if the user has explicitly asked to change only the libevent
     // flag and it failed.
-    uint32_t changedLogCategories = originalLogCategories ^ logCategories;
-    if (changedLogCategories & BCLog::LIBEVENT) {
-        if (!UpdateHTTPServerLogging(logCategories & BCLog::LIBEVENT)) {
-            logCategories &= ~BCLog::LIBEVENT;
-            if (changedLogCategories == BCLog::LIBEVENT) {
+    if (changed_log_categories & BCLog::LIBEVENT) {
+        if (!UpdateHTTPServerLogging(g_logger->WillLogCategory(BCLog::LIBEVENT))) {
+            g_logger->DisableCategory(BCLog::LIBEVENT);
+            if (changed_log_categories == BCLog::LIBEVENT) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "libevent logging cannot be updated when using libevent before v2.1.1.");
             }
         }

From 1eac317f25b905e97e311130ab19c3b0d257fc04 Mon Sep 17 00:00:00 2001
From: Jim Posen <jimpo@coinbase.com>
Date: Wed, 11 Apr 2018 14:06:35 -0700
Subject: [PATCH 4/6] util: Refactor GetLogCategory.

Changing parameter types from pointers to references and uint32_t to
BCLog::LogFlags simplies calling code.
---
 src/init.cpp     | 10 ++--------
 src/logging.cpp  | 51 ++++++++++++++++++++++++++++++------------------
 src/logging.h    |  8 ++++++--
 src/rpc/misc.cpp | 17 ++++++++--------
 4 files changed, 48 insertions(+), 38 deletions(-)

diff --git a/src/init.cpp b/src/init.cpp
index ccaa09a85..c0eb746d7 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -963,24 +963,18 @@ bool AppInitParameterInteraction()
         if (std::none_of(categories.begin(), categories.end(),
             [](std::string cat){return cat == "0" || cat == "none";})) {
             for (const auto& cat : categories) {
-                uint32_t flag = 0;
-                if (!GetLogCategory(&flag, &cat)) {
+                if (!g_logger->EnableCategory(cat)) {
                     InitWarning(strprintf(_("Unsupported logging category %s=%s."), "-debug", cat));
-                    continue;
                 }
-                g_logger->EnableCategory(static_cast<BCLog::LogFlags>(flag));
             }
         }
     }
 
     // Now remove the logging categories which were explicitly excluded
     for (const std::string& cat : gArgs.GetArgs("-debugexclude")) {
-        uint32_t flag = 0;
-        if (!GetLogCategory(&flag, &cat)) {
+        if (!g_logger->DisableCategory(cat)) {
             InitWarning(strprintf(_("Unsupported logging category %s=%s."), "-debugexclude", cat));
-            continue;
         }
-        g_logger->DisableCategory(static_cast<BCLog::LogFlags>(flag));
     }
 
     // Check for -debugnet
diff --git a/src/logging.cpp b/src/logging.cpp
index 7604c0fd9..dc1ed0afb 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -5,7 +5,6 @@
 
 #include <logging.h>
 #include <util.h>
-#include <utilstrencodings.h>
 
 const char * const DEFAULT_DEBUGLOGFILE = "debug.log";
 
@@ -64,11 +63,27 @@ void BCLog::Logger::EnableCategory(BCLog::LogFlags flag)
     logCategories |= flag;
 }
 
+bool BCLog::Logger::EnableCategory(const std::string& str)
+{
+    BCLog::LogFlags flag;
+    if (!GetLogCategory(flag, str)) return false;
+    EnableCategory(flag);
+    return true;
+}
+
 void BCLog::Logger::DisableCategory(BCLog::LogFlags flag)
 {
     logCategories &= ~flag;
 }
 
+bool BCLog::Logger::DisableCategory(const std::string& str)
+{
+    BCLog::LogFlags flag;
+    if (!GetLogCategory(flag, str)) return false;
+    DisableCategory(flag);
+    return true;
+}
+
 bool BCLog::Logger::WillLogCategory(BCLog::LogFlags category) const
 {
     return (logCategories.load(std::memory_order_relaxed) & category) != 0;
@@ -81,7 +96,7 @@ bool BCLog::Logger::DefaultShrinkDebugFile() const
 
 struct CLogCategoryDesc
 {
-    uint32_t flag;
+    BCLog::LogFlags flag;
     std::string category;
 };
 
@@ -114,19 +129,17 @@ const CLogCategoryDesc LogCategories[] =
     {BCLog::ALL, "all"},
 };
 
-bool GetLogCategory(uint32_t *f, const std::string *str)
+bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str)
 {
-    if (f && str) {
-        if (*str == "") {
-            *f = BCLog::ALL;
+    if (str == "") {
+        flag = BCLog::ALL;
+        return true;
+    }
+    for (const CLogCategoryDesc& category_desc : LogCategories) {
+        if (category_desc.category == str) {
+            flag = category_desc.flag;
             return true;
         }
-        for (unsigned int i = 0; i < ARRAYLEN(LogCategories); i++) {
-            if (LogCategories[i].category == *str) {
-                *f = LogCategories[i].flag;
-                return true;
-            }
-        }
     }
     return false;
 }
@@ -135,11 +148,11 @@ std::string ListLogCategories()
 {
     std::string ret;
     int outcount = 0;
-    for (unsigned int i = 0; i < ARRAYLEN(LogCategories); i++) {
+    for (const CLogCategoryDesc& category_desc : LogCategories) {
         // Omit the special cases.
-        if (LogCategories[i].flag != BCLog::NONE && LogCategories[i].flag != BCLog::ALL) {
+        if (category_desc.flag != BCLog::NONE && category_desc.flag != BCLog::ALL) {
             if (outcount != 0) ret += ", ";
-            ret += LogCategories[i].category;
+            ret += category_desc.category;
             outcount++;
         }
     }
@@ -149,12 +162,12 @@ std::string ListLogCategories()
 std::vector<CLogCategoryActive> ListActiveLogCategories()
 {
     std::vector<CLogCategoryActive> ret;
-    for (unsigned int i = 0; i < ARRAYLEN(LogCategories); i++) {
+    for (const CLogCategoryDesc& category_desc : LogCategories) {
         // Omit the special cases.
-        if (LogCategories[i].flag != BCLog::NONE && LogCategories[i].flag != BCLog::ALL) {
+        if (category_desc.flag != BCLog::NONE && category_desc.flag != BCLog::ALL) {
             CLogCategoryActive catActive;
-            catActive.category = LogCategories[i].category;
-            catActive.active = LogAcceptCategory(LogCategories[i].flag);
+            catActive.category = category_desc.category;
+            catActive.active = LogAcceptCategory(category_desc.flag);
             ret.push_back(catActive);
         }
     }
diff --git a/src/logging.h b/src/logging.h
index 7a1c25233..b88c9d991 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -95,8 +95,12 @@ namespace BCLog {
         void ShrinkDebugFile();
 
         uint32_t GetCategoryMask() const { return logCategories.load(); }
+
         void EnableCategory(LogFlags flag);
+        bool EnableCategory(const std::string& str);
         void DisableCategory(LogFlags flag);
+        bool DisableCategory(const std::string& str);
+
         bool WillLogCategory(LogFlags category) const;
 
         bool DefaultShrinkDebugFile() const;
@@ -118,8 +122,8 @@ std::string ListLogCategories();
 /** Returns a vector of the active log categories. */
 std::vector<CLogCategoryActive> ListActiveLogCategories();
 
-/** Return true if str parses as a log category and set the flags in f */
-bool GetLogCategory(uint32_t *f, const std::string *str);
+/** Return true if str parses as a log category and set the flag */
+bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str);
 
 /** Get format string from VA_ARGS for error reporting */
 template<typename... Args> std::string FormatStringFromLogArgs(const char *fmt, const Args&... args) { return fmt; }
diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp
index 26bf21356..0c93108bc 100644
--- a/src/rpc/misc.cpp
+++ b/src/rpc/misc.cpp
@@ -349,18 +349,17 @@ UniValue getmemoryinfo(const JSONRPCRequest& request)
 void EnableOrDisableLogCategories(UniValue cats, bool enable) {
     cats = cats.get_array();
     for (unsigned int i = 0; i < cats.size(); ++i) {
-        uint32_t flag = 0;
         std::string cat = cats[i].get_str();
-        if (!GetLogCategory(&flag, &cat)) {
-            throw JSONRPCError(RPC_INVALID_PARAMETER, "unknown logging category " + cat);
-        }
-        if (flag == BCLog::NONE) {
-            return;
-        }
+
+        bool success;
         if (enable) {
-            g_logger->EnableCategory(static_cast<BCLog::LogFlags>(flag));
+            success = g_logger->EnableCategory(cat);
         } else {
-            g_logger->DisableCategory(static_cast<BCLog::LogFlags>(flag));
+            success = g_logger->DisableCategory(cat);
+        }
+
+        if (!success) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "unknown logging category " + cat);
         }
     }
 }

From 8e7b961388920144993d0bd56d93f89e5c60fbff Mon Sep 17 00:00:00 2001
From: Jim Posen <jimpo@coinbase.com>
Date: Fri, 20 Apr 2018 00:42:32 -0700
Subject: [PATCH 5/6] scripted-diff: Rename BCLog::Logger member variables.

-BEGIN VERIFY SCRIPT-
sed -i "s/fileout/m_fileout/" src/logging.h src/logging.cpp
sed -i "s/mutexDebugLog/m_file_mutex/" src/logging.h src/logging.cpp
sed -i "s/vMsgsBeforeOpenLog/m_msgs_before_open/" src/logging.h src/logging.cpp
sed -i "s/logCategories/m_categories/" src/logging.h src/logging.cpp
sed -i "s/fPrintToConsole/m_print_to_console/" src/logging.h src/logging.cpp src/init.cpp
sed -i "s/fPrintToDebugLog/m_print_to_file/" src/logging.h src/logging.cpp src/init.cpp src/test/test_bitcoin.cpp src/bench/bench_bitcoin.cpp
sed -i "s/fLogTimestamps/m_log_timestamps/" src/logging.h src/logging.cpp src/init.cpp
sed -i "s/fLogTimeMicros/m_log_time_micros/" src/logging.h src/logging.cpp src/init.cpp
sed -i "s/fReopenDebugLog/m_reopen_file/" src/logging.h src/logging.cpp src/init.cpp
sed -i "s/fStartedNewLine/m_started_new_line/" src/logging.h src/logging.cpp
-END VERIFY SCRIPT-
---
 src/bench/bench_bitcoin.cpp |  2 +-
 src/init.cpp                | 14 +++++-----
 src/logging.cpp             | 54 ++++++++++++++++++-------------------
 src/logging.h               | 26 +++++++++---------
 src/test/test_bitcoin.cpp   |  2 +-
 5 files changed, 49 insertions(+), 49 deletions(-)

diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp
index f08c099c1..c1fbeb8d1 100644
--- a/src/bench/bench_bitcoin.cpp
+++ b/src/bench/bench_bitcoin.cpp
@@ -46,7 +46,7 @@ main(int argc, char** argv)
     RandomInit();
     ECC_Start();
     SetupEnvironment();
-    g_logger->fPrintToDebugLog = false; // don't want to write to debug.log file
+    g_logger->m_print_to_file = false; // don't want to write to debug.log file
 
     int64_t evaluations = gArgs.GetArg("-evals", DEFAULT_BENCH_EVALUATIONS);
     std::string regex_filter = gArgs.GetArg("-filter", DEFAULT_BENCH_FILTER);
diff --git a/src/init.cpp b/src/init.cpp
index c0eb746d7..7bc2f6302 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -305,7 +305,7 @@ static void HandleSIGTERM(int)
 
 static void HandleSIGHUP(int)
 {
-    g_logger->fReopenDebugLog = true;
+    g_logger->m_reopen_file = true;
 }
 
 #ifndef WIN32
@@ -831,10 +831,10 @@ void InitLogging()
     // debug.log.
     LogPrintf("\n\n\n\n\n");
 
-    g_logger->fPrintToConsole = gArgs.GetBoolArg("-printtoconsole", !gArgs.GetBoolArg("-daemon", false));
-    g_logger->fPrintToDebugLog = !gArgs.IsArgNegated("-debuglogfile");
-    g_logger->fLogTimestamps = gArgs.GetBoolArg("-logtimestamps", DEFAULT_LOGTIMESTAMPS);
-    g_logger->fLogTimeMicros = gArgs.GetBoolArg("-logtimemicros", DEFAULT_LOGTIMEMICROS);
+    g_logger->m_print_to_console = gArgs.GetBoolArg("-printtoconsole", !gArgs.GetBoolArg("-daemon", false));
+    g_logger->m_print_to_file = !gArgs.IsArgNegated("-debuglogfile");
+    g_logger->m_log_timestamps = gArgs.GetBoolArg("-logtimestamps", DEFAULT_LOGTIMESTAMPS);
+    g_logger->m_log_time_micros = gArgs.GetBoolArg("-logtimemicros", DEFAULT_LOGTIMEMICROS);
 
     fLogIPs = gArgs.GetBoolArg("-logips", DEFAULT_LOGIPS);
 
@@ -1225,7 +1225,7 @@ bool AppInitMain()
 #ifndef WIN32
     CreatePidFile(GetPidFile(), getpid());
 #endif
-    if (g_logger->fPrintToDebugLog) {
+    if (g_logger->m_print_to_file) {
         if (gArgs.GetBoolArg("-shrinkdebugfile", g_logger->DefaultShrinkDebugFile())) {
             // Do this first since it both loads a bunch of debug.log into memory,
             // and because this needs to happen before any other debug.log printing
@@ -1237,7 +1237,7 @@ bool AppInitMain()
         }
     }
 
-    if (!g_logger->fLogTimestamps)
+    if (!g_logger->m_log_timestamps)
         LogPrintf("Startup time: %s\n", FormatISO8601DateTime(GetTime()));
     LogPrintf("Default data directory %s\n", GetDefaultDataDir().string());
     LogPrintf("Using data directory %s\n", GetDataDir().string());
diff --git a/src/logging.cpp b/src/logging.cpp
index dc1ed0afb..b7c682c94 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -38,21 +38,21 @@ fs::path BCLog::Logger::GetDebugLogPath() const
 
 bool BCLog::Logger::OpenDebugLog()
 {
-    std::lock_guard<std::mutex> scoped_lock(mutexDebugLog);
+    std::lock_guard<std::mutex> scoped_lock(m_file_mutex);
 
-    assert(fileout == nullptr);
+    assert(m_fileout == nullptr);
     fs::path pathDebug = GetDebugLogPath();
 
-    fileout = fsbridge::fopen(pathDebug, "a");
-    if (!fileout) {
+    m_fileout = fsbridge::fopen(pathDebug, "a");
+    if (!m_fileout) {
         return false;
     }
 
-    setbuf(fileout, nullptr); // unbuffered
+    setbuf(m_fileout, nullptr); // unbuffered
     // dump buffered messages from before we opened the log
-    while (!vMsgsBeforeOpenLog.empty()) {
-        FileWriteStr(vMsgsBeforeOpenLog.front(), fileout);
-        vMsgsBeforeOpenLog.pop_front();
+    while (!m_msgs_before_open.empty()) {
+        FileWriteStr(m_msgs_before_open.front(), m_fileout);
+        m_msgs_before_open.pop_front();
     }
 
     return true;
@@ -60,7 +60,7 @@ bool BCLog::Logger::OpenDebugLog()
 
 void BCLog::Logger::EnableCategory(BCLog::LogFlags flag)
 {
-    logCategories |= flag;
+    m_categories |= flag;
 }
 
 bool BCLog::Logger::EnableCategory(const std::string& str)
@@ -73,7 +73,7 @@ bool BCLog::Logger::EnableCategory(const std::string& str)
 
 void BCLog::Logger::DisableCategory(BCLog::LogFlags flag)
 {
-    logCategories &= ~flag;
+    m_categories &= ~flag;
 }
 
 bool BCLog::Logger::DisableCategory(const std::string& str)
@@ -86,12 +86,12 @@ bool BCLog::Logger::DisableCategory(const std::string& str)
 
 bool BCLog::Logger::WillLogCategory(BCLog::LogFlags category) const
 {
-    return (logCategories.load(std::memory_order_relaxed) & category) != 0;
+    return (m_categories.load(std::memory_order_relaxed) & category) != 0;
 }
 
 bool BCLog::Logger::DefaultShrinkDebugFile() const
 {
-    return logCategories == BCLog::NONE;
+    return m_categories == BCLog::NONE;
 }
 
 struct CLogCategoryDesc
@@ -178,13 +178,13 @@ std::string BCLog::Logger::LogTimestampStr(const std::string &str)
 {
     std::string strStamped;
 
-    if (!fLogTimestamps)
+    if (!m_log_timestamps)
         return str;
 
-    if (fStartedNewLine) {
+    if (m_started_new_line) {
         int64_t nTimeMicros = GetTimeMicros();
         strStamped = FormatISO8601DateTime(nTimeMicros/1000000);
-        if (fLogTimeMicros) {
+        if (m_log_time_micros) {
             strStamped.pop_back();
             strStamped += strprintf(".%06dZ", nTimeMicros%1000000);
         }
@@ -197,9 +197,9 @@ std::string BCLog::Logger::LogTimestampStr(const std::string &str)
         strStamped = str;
 
     if (!str.empty() && str[str.size()-1] == '\n')
-        fStartedNewLine = true;
+        m_started_new_line = true;
     else
-        fStartedNewLine = false;
+        m_started_new_line = false;
 
     return strStamped;
 }
@@ -210,30 +210,30 @@ int BCLog::Logger::LogPrintStr(const std::string &str)
 
     std::string strTimestamped = LogTimestampStr(str);
 
-    if (fPrintToConsole) {
+    if (m_print_to_console) {
         // print to console
         ret = fwrite(strTimestamped.data(), 1, strTimestamped.size(), stdout);
         fflush(stdout);
     }
-    if (fPrintToDebugLog) {
-        std::lock_guard<std::mutex> scoped_lock(mutexDebugLog);
+    if (m_print_to_file) {
+        std::lock_guard<std::mutex> scoped_lock(m_file_mutex);
 
         // buffer if we haven't opened the log yet
-        if (fileout == nullptr) {
+        if (m_fileout == nullptr) {
             ret = strTimestamped.length();
-            vMsgsBeforeOpenLog.push_back(strTimestamped);
+            m_msgs_before_open.push_back(strTimestamped);
         }
         else
         {
             // reopen the log file, if requested
-            if (fReopenDebugLog) {
-                fReopenDebugLog = false;
+            if (m_reopen_file) {
+                m_reopen_file = false;
                 fs::path pathDebug = GetDebugLogPath();
-                if (fsbridge::freopen(pathDebug,"a",fileout) != nullptr)
-                    setbuf(fileout, nullptr); // unbuffered
+                if (fsbridge::freopen(pathDebug,"a",m_fileout) != nullptr)
+                    setbuf(m_fileout, nullptr); // unbuffered
             }
 
-            ret = FileWriteStr(strTimestamped, fileout);
+            ret = FileWriteStr(strTimestamped, m_fileout);
         }
     }
     return ret;
diff --git a/src/logging.h b/src/logging.h
index b88c9d991..249d5debe 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -59,42 +59,42 @@ namespace BCLog {
     class Logger
     {
     private:
-        FILE* fileout = nullptr;
-        std::mutex mutexDebugLog;
-        std::list<std::string> vMsgsBeforeOpenLog;
+        FILE* m_fileout = nullptr;
+        std::mutex m_file_mutex;
+        std::list<std::string> m_msgs_before_open;
 
         /**
-         * fStartedNewLine is a state variable that will suppress printing of
+         * m_started_new_line is a state variable that will suppress printing of
          * the timestamp when multiple calls are made that don't end in a
          * newline.
          */
-        std::atomic_bool fStartedNewLine{true};
+        std::atomic_bool m_started_new_line{true};
 
         /** Log categories bitfield. */
-        std::atomic<uint32_t> logCategories{0};
+        std::atomic<uint32_t> m_categories{0};
 
         std::string LogTimestampStr(const std::string& str);
 
     public:
-        bool fPrintToConsole = false;
-        bool fPrintToDebugLog = true;
+        bool m_print_to_console = false;
+        bool m_print_to_file = true;
 
-        bool fLogTimestamps = DEFAULT_LOGTIMESTAMPS;
-        bool fLogTimeMicros = DEFAULT_LOGTIMEMICROS;
+        bool m_log_timestamps = DEFAULT_LOGTIMESTAMPS;
+        bool m_log_time_micros = DEFAULT_LOGTIMEMICROS;
 
-        std::atomic<bool> fReopenDebugLog{false};
+        std::atomic<bool> m_reopen_file{false};
 
         /** Send a string to the log output */
         int LogPrintStr(const std::string &str);
 
         /** Returns whether logs will be written to any output */
-        bool Enabled() const { return fPrintToConsole || fPrintToDebugLog; }
+        bool Enabled() const { return m_print_to_console || m_print_to_file; }
 
         fs::path GetDebugLogPath() const;
         bool OpenDebugLog();
         void ShrinkDebugFile();
 
-        uint32_t GetCategoryMask() const { return logCategories.load(); }
+        uint32_t GetCategoryMask() const { return m_categories.load(); }
 
         void EnableCategory(LogFlags flag);
         bool EnableCategory(const std::string& str);
diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp
index fa59a9ce3..eea180488 100644
--- a/src/test/test_bitcoin.cpp
+++ b/src/test/test_bitcoin.cpp
@@ -47,7 +47,7 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName)
         SetupNetworking();
         InitSignatureCache();
         InitScriptExecutionCache();
-        g_logger->fPrintToDebugLog = false; // don't want to write to debug.log file
+        g_logger->m_print_to_file = false; // don't want to write to debug.log file
         fCheckBlockIndex = true;
         SelectParams(chainName);
         noui_connect();

From 8c2d695c4a45bdd9378c7970b0fcba6e1efc01f9 Mon Sep 17 00:00:00 2001
From: Jim Posen <jimpo@coinbase.com>
Date: Fri, 20 Apr 2018 01:11:44 -0700
Subject: [PATCH 6/6] util: Store debug log file path in BCLog::Logger member.

This breaks the cyclic between logging and util.
---
 src/bench/bench_bitcoin.cpp |  1 -
 src/init.cpp                |  6 ++++--
 src/logging.cpp             | 25 ++++++++++---------------
 src/logging.h               |  4 ++--
 src/test/test_bitcoin.cpp   |  1 -
 5 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/src/bench/bench_bitcoin.cpp b/src/bench/bench_bitcoin.cpp
index c1fbeb8d1..c1f333983 100644
--- a/src/bench/bench_bitcoin.cpp
+++ b/src/bench/bench_bitcoin.cpp
@@ -46,7 +46,6 @@ main(int argc, char** argv)
     RandomInit();
     ECC_Start();
     SetupEnvironment();
-    g_logger->m_print_to_file = false; // don't want to write to debug.log file
 
     int64_t evaluations = gArgs.GetArg("-evals", DEFAULT_BENCH_EVALUATIONS);
     std::string regex_filter = gArgs.GetArg("-filter", DEFAULT_BENCH_FILTER);
diff --git a/src/init.cpp b/src/init.cpp
index 7bc2f6302..6423d8702 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -826,13 +826,15 @@ static std::string ResolveErrMsg(const char * const optname, const std::string&
  */
 void InitLogging()
 {
+    g_logger->m_print_to_file = !gArgs.IsArgNegated("-debuglogfile");
+    g_logger->m_file_path = AbsPathForConfigVal(gArgs.GetArg("-debuglogfile", DEFAULT_DEBUGLOGFILE));
+
     // Add newlines to the logfile to distinguish this execution from the last
     // one; called before console logging is set up, so this is only sent to
     // debug.log.
     LogPrintf("\n\n\n\n\n");
 
     g_logger->m_print_to_console = gArgs.GetBoolArg("-printtoconsole", !gArgs.GetBoolArg("-daemon", false));
-    g_logger->m_print_to_file = !gArgs.IsArgNegated("-debuglogfile");
     g_logger->m_log_timestamps = gArgs.GetBoolArg("-logtimestamps", DEFAULT_LOGTIMESTAMPS);
     g_logger->m_log_time_micros = gArgs.GetBoolArg("-logtimemicros", DEFAULT_LOGTIMEMICROS);
 
@@ -1233,7 +1235,7 @@ bool AppInitMain()
         }
         if (!g_logger->OpenDebugLog()) {
             return InitError(strprintf("Could not open debug log file %s",
-                                       g_logger->GetDebugLogPath().string()));
+                                       g_logger->m_file_path.string()));
         }
     }
 
diff --git a/src/logging.cpp b/src/logging.cpp
index b7c682c94..10a3b1895 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -4,7 +4,7 @@
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
 #include <logging.h>
-#include <util.h>
+#include <utiltime.h>
 
 const char * const DEFAULT_DEBUGLOGFILE = "debug.log";
 
@@ -30,20 +30,14 @@ static int FileWriteStr(const std::string &str, FILE *fp)
     return fwrite(str.data(), 1, str.size(), fp);
 }
 
-fs::path BCLog::Logger::GetDebugLogPath() const
-{
-    fs::path logfile(gArgs.GetArg("-debuglogfile", DEFAULT_DEBUGLOGFILE));
-    return AbsPathForConfigVal(logfile);
-}
-
 bool BCLog::Logger::OpenDebugLog()
 {
     std::lock_guard<std::mutex> scoped_lock(m_file_mutex);
 
     assert(m_fileout == nullptr);
-    fs::path pathDebug = GetDebugLogPath();
+    assert(!m_file_path.empty());
 
-    m_fileout = fsbridge::fopen(pathDebug, "a");
+    m_fileout = fsbridge::fopen(m_file_path, "a");
     if (!m_fileout) {
         return false;
     }
@@ -228,8 +222,7 @@ int BCLog::Logger::LogPrintStr(const std::string &str)
             // reopen the log file, if requested
             if (m_reopen_file) {
                 m_reopen_file = false;
-                fs::path pathDebug = GetDebugLogPath();
-                if (fsbridge::freopen(pathDebug,"a",m_fileout) != nullptr)
+                if (fsbridge::freopen(m_file_path,"a",m_fileout) != nullptr)
                     setbuf(m_fileout, nullptr); // unbuffered
             }
 
@@ -243,14 +236,16 @@ void BCLog::Logger::ShrinkDebugFile()
 {
     // Amount of debug.log to save at end when shrinking (must fit in memory)
     constexpr size_t RECENT_DEBUG_HISTORY_SIZE = 10 * 1000000;
+
+    assert(!m_file_path.empty());
+
     // Scroll debug.log if it's getting too big
-    fs::path pathLog = GetDebugLogPath();
-    FILE* file = fsbridge::fopen(pathLog, "r");
+    FILE* file = fsbridge::fopen(m_file_path, "r");
 
     // Special files (e.g. device nodes) may not have a size.
     size_t log_size = 0;
     try {
-        log_size = fs::file_size(pathLog);
+        log_size = fs::file_size(m_file_path);
     } catch (boost::filesystem::filesystem_error &) {}
 
     // If debug.log file is more than 10% bigger the RECENT_DEBUG_HISTORY_SIZE
@@ -263,7 +258,7 @@ void BCLog::Logger::ShrinkDebugFile()
         int nBytes = fread(vch.data(), 1, vch.size(), file);
         fclose(file);
 
-        file = fsbridge::fopen(pathLog, "w");
+        file = fsbridge::fopen(m_file_path, "w");
         if (file)
         {
             fwrite(vch.data(), 1, nBytes, file);
diff --git a/src/logging.h b/src/logging.h
index 249d5debe..1f2be6016 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -77,11 +77,12 @@ namespace BCLog {
 
     public:
         bool m_print_to_console = false;
-        bool m_print_to_file = true;
+        bool m_print_to_file = false;
 
         bool m_log_timestamps = DEFAULT_LOGTIMESTAMPS;
         bool m_log_time_micros = DEFAULT_LOGTIMEMICROS;
 
+        fs::path m_file_path;
         std::atomic<bool> m_reopen_file{false};
 
         /** Send a string to the log output */
@@ -90,7 +91,6 @@ namespace BCLog {
         /** Returns whether logs will be written to any output */
         bool Enabled() const { return m_print_to_console || m_print_to_file; }
 
-        fs::path GetDebugLogPath() const;
         bool OpenDebugLog();
         void ShrinkDebugFile();
 
diff --git a/src/test/test_bitcoin.cpp b/src/test/test_bitcoin.cpp
index eea180488..fe816a6f7 100644
--- a/src/test/test_bitcoin.cpp
+++ b/src/test/test_bitcoin.cpp
@@ -47,7 +47,6 @@ BasicTestingSetup::BasicTestingSetup(const std::string& chainName)
         SetupNetworking();
         InitSignatureCache();
         InitScriptExecutionCache();
-        g_logger->m_print_to_file = false; // don't want to write to debug.log file
         fCheckBlockIndex = true;
         SelectParams(chainName);
         noui_connect();