Repository: bitcoin/bitcoin
File: src/logging.cpp (C++)

Code before:
    {"net", BCLog::NET},
    {"tor", BCLog::TOR},
    {"mempool", BCLog::MEMPOOL},
    {"http", BCLog::HTTP},
    {"bench", BCLog::BENCH},
    {"zmq", BCLog::ZMQ},
    {"walletdb", BCLog::WALLETDB},
    {"rpc", BCLog::RPC},
    {"estimatefee", BCLog::ESTIMATEFEE},
    {"addrman", BCLog::ADDRMAN},
    {"selectcoins", BCLog::SELECTCOINS},
    {"reindex", BCLog::REINDEX},
    {"cmpctblock", BCLog::CMPCTBLOCK},
    {"rand", BCLog::RAND},
    {"prune", BCLog::PRUNE},
    {"proxy", BCLog::PROXY},
    {"mempoolrej", BCLog::MEMPOOLREJ},
    {"libevent", BCLog::LIBEVENT},
    {"coindb", BCLog::COINDB},
    {"qt", BCLog::QT},
    {"leveldb", BCLog::LEVELDB},
    {"validation", BCLog::VALIDATION},
    {"i2p", BCLog::I2P},
    {"ipc", BCLog::IPC},
#ifdef DEBUG_LOCKCONTENTION
    {"lock", BCLog::LOCK},
#endif
    {"blockstorage", BCLog::BLOCKSTORAGE},
    {"txreconciliation", BCLog::TXRECONCILIATION},
    {"scan", BCLog::SCAN},
    {"txpackages", BCLog::TXPACKAGES},
    {"1", BCLog::ALL},
    {"all", BCLog::ALL},
};

static const std::unordered_map<BCLog::LogFlags, std::string> LOG_CATEGORIES_BY_FLAG{
    // Swap keys and values from LOG_CATEGORIES_BY_STR.
    [](const auto& in) {
        std::unordered_map<BCLog::LogFlags, std::string> out;
        for (const auto& [k, v] : in) {
            switch (v) {
            case BCLog::NONE: out.emplace(BCLog::NONE, ""); break;
            case BCLog::ALL: out.emplace(BCLog::ALL, "all"); break;
            default: out.emplace(v, k);
            }
        }
        return out;
    }(LOG_CATEGORIES_BY_STR)
};

bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str)

Review comment (bug):
In commit "logging: move special cases to GetLogCategory()" (9fb321ad2b0176955718317b796715250eac01f4)

Would be good if commit title had a "refactor:" prefix so it is clear this is not intended to change behavior.

Also, I think this commit should remove ALL and NONE cases from the map, so special cases are handled completely within the GetLogCategory() and LogCategoryToStr() functions, and there are no more special cases in the map and LogCategoriesList(). Would suggest:

```diff
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -142,7 +142,6 @@ bool BCLog::Logger::DefaultShrinkDebugFile() const
 }
 
 static const std::map<std::string, BCLog::LogFlags> LOG_CATEGORIES_BY_STR{
-    {"", BCLog::NONE},
     {"net", BCLog::NET},
     {"tor", BCLog::TOR},
     {"mempool", BCLog::MEMPOOL},
@@ -174,7 +173,6 @@ static const std::map<std::string, BCLog::LogFlags> LOG_CATEGORIES_BY_STR{
     {"txreconciliation", BCLog::TXRECONCILIATION},
     {"scan", BCLog::SCAN},
     {"txpackages", BCLog::TXPACKAGES},
-    {"all", BCLog::ALL},
 };
 
 static const std::unordered_map<BCLog::LogFlags, std::string> LOG_CATEGORIES_BY_FLAG{
@@ -191,11 +189,10 @@ static const std::unordered_map<BCLog::LogFlags, std::string> LOG_CATEGORIES_BY_
 
 bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str)
 {
-    if (str.empty() || str == "1") {
+    if (str.empty() || str == "1" || str == "all") {
         flag = BCLog::ALL;
         return true;
-    }
-    if (str == "0") {
+    } else if (str == "0") {
         flag = BCLog::NONE;
         return true;
     }
@@ -226,6 +223,11 @@ std::string BCLog::Logger::LogLevelToStr(BCLog::Level level)
 
 std::string LogCategoryToStr(BCLog::LogFlags category)
 {
+    if (category == BCLog::NONE) {
+        return "";
+    } else if (category == BCLog::ALL) {
+        return "all";
+    }
     auto it = LOG_CATEGORIES_BY_FLAG.find(category);
     assert(it != LOG_CATEGORIES_BY_FLAG.end());
     return it->second;
@@ -252,9 +254,7 @@ std::vector<LogCategory> BCLog::Logger::LogCategoriesList() const
 {
     std::vector<LogCategory> ret;
     for (const auto& [category, flag] : LOG_CATEGORIES_BY_STR) {
-        if (flag != BCLog::NONE && flag != BCLog::ALL) {
-            ret.push_back(LogCategory{.category = category, .active = WillLogCategory(flag)});
-        }
+        ret.push_back(LogCategory{.category = category, .active = WillLogCategory(flag)});
     }
     return ret;
 }

```

And would suggest a commit message like:

```
logging, refactor: make category special cases explicit

Make special cases explicit in GetLogCategory() and LogCategoryToStr()
functions. Simplify the LOG_CATEGORIES_BY_STR and LOG_CATEGORIES_BY_FLAG
mappings and LogCategoriesList() function.
```


Code after:
    {"mempool", BCLog::MEMPOOL},
    {"http", BCLog::HTTP},
    {"bench", BCLog::BENCH},
    {"zmq", BCLog::ZMQ},
    {"walletdb", BCLog::WALLETDB},
    {"rpc", BCLog::RPC},
    {"estimatefee", BCLog::ESTIMATEFEE},
    {"addrman", BCLog::ADDRMAN},
    {"selectcoins", BCLog::SELECTCOINS},
    {"reindex", BCLog::REINDEX},
    {"cmpctblock", BCLog::CMPCTBLOCK},
    {"rand", BCLog::RAND},
    {"prune", BCLog::PRUNE},
    {"proxy", BCLog::PROXY},
    {"mempoolrej", BCLog::MEMPOOLREJ},
    {"libevent", BCLog::LIBEVENT},
    {"coindb", BCLog::COINDB},
    {"qt", BCLog::QT},
    {"leveldb", BCLog::LEVELDB},
    {"validation", BCLog::VALIDATION},
    {"i2p", BCLog::I2P},
    {"ipc", BCLog::IPC},
#ifdef DEBUG_LOCKCONTENTION
    {"lock", BCLog::LOCK},
#endif
    {"blockstorage", BCLog::BLOCKSTORAGE},
    {"txreconciliation", BCLog::TXRECONCILIATION},
    {"scan", BCLog::SCAN},
    {"txpackages", BCLog::TXPACKAGES},
};

static const std::unordered_map<BCLog::LogFlags, std::string> LOG_CATEGORIES_BY_FLAG{
    // Swap keys and values from LOG_CATEGORIES_BY_STR.
    [](const auto& in) {
        std::unordered_map<BCLog::LogFlags, std::string> out;
        for (const auto& [k, v] : in) {
            const bool inserted{out.emplace(v, k).second};
            assert(inserted);
        }
        return out;
    }(LOG_CATEGORIES_BY_STR)
};

bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str)
{
    if (str.empty() || str == "1" || str == "all") {
        flag = BCLog::ALL;
        return true;
    }
    auto it = LOG_CATEGORIES_BY_STR.find(str);
    if (it != LOG_CATEGORIES_BY_STR.end()) {