Merge #18669: log: Use Join() helper when listing log categories

faec063887 log: Use Join() helper when listing log categories (MarcoFalke)

Pull request description:

  This removes the global `ListLogCategories` and replaces it with a one-line member function `LogCategoriesString`, which just calls `Join`.

  Should be a straightforward refactor to get rid of a few LOC.

ACKs for top commit:
  laanwj:
    ACK faec063887
  promag:
    ACK faec063887, I also think it's fine as it is (re https://github.com/bitcoin/bitcoin/pull/18669#discussion_r412944724).

Tree-SHA512: 2f51f9ce1246eda5630015f3a869e36953c7eb34f311baad576b92d7829e4e88051c6189436271cd0a13732a49698506345b446b98fd28e58edfb5b62169f1c9

src/init.cpp

@@ -520,7 +520,8 @@ void SetupServerArgs(NodeContext& node)
     gArgs.AddArg("-limitdescendantsize=<n>", strprintf("Do not accept transactions if any ancestor would have more than <n> kilobytes of in-mempool descendants (default: %u).", DEFAULT_DESCENDANT_SIZE_LIMIT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-addrmantest", "Allows to test address relay on localhost", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-debug=<category>", "Output debugging information (default: -nodebug, supplying <category> is optional). "
-        "If <category> is not supplied or if <category> = 1, output all debugging information. <category> can be: " + ListLogCategories() + ".", ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
+        "If <category> is not supplied or if <category> = 1, output all debugging information. <category> can be: " + LogInstance().LogCategoriesString() + ".",
+        ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-debugexclude=<category>", strprintf("Exclude debugging information for a category. Can be used in conjunction with -debug=1 to output debug logs for all categories except one or more specified categories."), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-logips", strprintf("Include IP addresses in debug output (default: %u)", DEFAULT_LOGIPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);
     gArgs.AddArg("-logtimestamps", strprintf("Prepend debug output with timestamp (default: %u)", DEFAULT_LOGTIMESTAMPS), ArgsManager::ALLOW_ANY, OptionsCategory::DEBUG_TEST);

src/logging.cpp

@@ -182,30 +182,15 @@ bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str)
     return false;
 }
 
-std::string ListLogCategories()
+std::vector<LogCategory> BCLog::Logger::LogCategoriesList()
 {
-    std::string ret;
+    std::vector<LogCategory> ret;
-    int outcount = 0;
-    for (const CLogCategoryDesc& category_desc : LogCategories) {
-        // Omit the special cases.
-        if (category_desc.flag != BCLog::NONE && category_desc.flag != BCLog::ALL) {
-            if (outcount != 0) ret += ", ";
-            ret += category_desc.category;
-            outcount++;
-        }
-    }
-    return ret;
-}
-
-std::vector<CLogCategoryActive> ListActiveLogCategories()
-{
-    std::vector<CLogCategoryActive> ret;
     for (const CLogCategoryDesc& category_desc : LogCategories) {
         // Omit the special cases.
         if (category_desc.flag != BCLog::NONE && category_desc.flag != BCLog::ALL) {
-            CLogCategoryActive catActive;
+            LogCategory catActive;
             catActive.category = category_desc.category;
-            catActive.active = LogAcceptCategory(category_desc.flag);
+            catActive.active = WillLogCategory(category_desc.flag);
             ret.push_back(catActive);
         }
     }

src/logging.h

@@ -8,6 +8,7 @@
 
 #include <fs.h>
 #include <tinyformat.h>
+#include <util/string.h>
 
 #include <atomic>
 #include <cstdint>
@@ -24,8 +25,7 @@ extern const char * const DEFAULT_DEBUGLOGFILE;
 
 extern bool fLogIPs;
 
-struct CLogCategoryActive
+struct LogCategory {
-{
     std::string category;
     bool active;
 };
@@ -132,6 +132,13 @@ namespace BCLog {
         bool DisableCategory(const std::string& str);
 
         bool WillLogCategory(LogFlags category) const;
+        /** Returns a vector of the log categories */
+        std::vector<LogCategory> LogCategoriesList();
+        /** Returns a string with the log categories */
+        std::string LogCategoriesString()
+        {
+            return Join(LogCategoriesList(), ", ", [&](const LogCategory& i) { return i.category; });
+        };
 
         bool DefaultShrinkDebugFile() const;
     };
@@ -146,12 +153,6 @@ static inline bool LogAcceptCategory(BCLog::LogFlags category)
     return LogInstance().WillLogCategory(category);
 }
 
-/** Returns a string with the log categories. */
-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 flag */
 bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str);
 

src/rpc/misc.cpp

@@ -516,7 +516,7 @@ UniValue logging(const JSONRPCRequest& request)
             "When called with arguments, adds or removes categories from debug logging and return the lists above.\n"
             "The arguments are evaluated in order \"include\", \"exclude\".\n"
             "If an item is both included and excluded, it will thus end up being excluded.\n"
-            "The valid logging categories are: " + ListLogCategories() + "\n"
+            "The valid logging categories are: " + LogInstance().LogCategoriesString() + "\n"
             "In addition, the following are available as category names with special meanings:\n"
             "  - \"all\",  \"1\" : represent all logging categories.\n"
             "  - \"none\", \"0\" : even if other logging categories are specified, ignore all of them.\n"
@@ -568,8 +568,7 @@ UniValue logging(const JSONRPCRequest& request)
     }
 
     UniValue result(UniValue::VOBJ);
-    std::vector<CLogCategoryActive> vLogCatActive = ListActiveLogCategories();
+    for (const auto& logCatActive : LogInstance().LogCategoriesList()) {
-    for (const auto& logCatActive : vLogCatActive) {
         result.pushKV(logCatActive.category, logCatActive.active);
     }