Could this naming cleanup be split before the atomic-mask change?
The current code already has one threshold that means "levels at or above this always log". Giving that threshold a shared name first would make the later mask change smaller, and the startup parsing checks could use the same name instead of hardcoding Level::Info.
diff --git a/src/logging.cpp b/src/logging.cpp
index 3373063c05..e0c2b2cef2 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -21,7 +21,6 @@ using util::Join;
using util::RemovePrefixView;
const char * const DEFAULT_DEBUGLOGFILE = "debug.log";
-constexpr auto MAX_USER_SETABLE_SEVERITY_LEVEL{BCLog::Level::Info};
BCLog::Logger& LogInstance()
{
@@ -162,7 +161,7 @@ bool BCLog::Logger::WillLogCategoryLevel(BCLog::LogFlags category, BCLog::Level
{
// Log messages at Info, Warning and Error level unconditionally, so that
// important troubleshooting information doesn't get lost.
- if (level >= BCLog::Level::Info) return true;
+ if (level >= BCLog::UNCONDITIONAL_LOG_LEVEL) return true;
if (!WillLogCategory(category)) return false;
@@ -263,7 +262,7 @@ static std::string LogCategoryToStr(BCLog::LogFlags category)
return it->second;
}
-static std::optional<BCLog::Level> GetLogLevel(std::string_view level_str)
+std::optional<BCLog::Level> BCLog::Logger::GetLogLevel(std::string_view level_str)
{
if (level_str == "trace") {
return BCLog::Level::Trace;
@@ -586,7 +585,7 @@ bool BCLog::LogRateLimiter::Stats::Consume(uint64_t bytes)
bool BCLog::Logger::SetLogLevel(std::string_view level_str)
{
const auto level = GetLogLevel(level_str);
- if (!level.has_value() || level.value() > MAX_USER_SETABLE_SEVERITY_LEVEL) return false;
+ if (!level.has_value() || level.value() > BCLog::UNCONDITIONAL_LOG_LEVEL) return false;
m_log_level = level.value();
return true;
}
@@ -597,7 +596,7 @@ bool BCLog::Logger::SetCategoryLogLevel(std::string_view category_str, std::stri
if (!flag) return false;
const auto level = GetLogLevel(level_str);
- if (!level.has_value() || level.value() > MAX_USER_SETABLE_SEVERITY_LEVEL) return false;
+ if (!level.has_value() || level.value() > BCLog::UNCONDITIONAL_LOG_LEVEL) return false;
STDLOCK(m_cs);
m_category_log_levels[*flag] = level.value();
diff --git a/src/logging.h b/src/logging.h
index 67f5d6ef00..041e341d3a 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -62,6 +62,8 @@ struct LogCategory {
namespace BCLog {
constexpr auto DEFAULT_LOG_LEVEL{Level::Debug};
+ //! Log messages at this severity level and above are always emitted regardless of category settings.
+ constexpr auto UNCONDITIONAL_LOG_LEVEL{Level::Info};
constexpr size_t DEFAULT_MAX_LOG_BUFFER{1'000'000}; // buffer up to 1MB of log data prior to StartLogging
constexpr uint64_t RATELIMIT_MAX_BYTES{1_MiB}; // maximum number of bytes per source location that can be logged within the RATELIMIT_WINDOW
constexpr auto RATELIMIT_WINDOW{1h}; // time window after which log ratelimit stats are reset
@@ -271,6 +273,8 @@ namespace BCLog {
//! Returns a string with all user-selectable log levels.
std::string LogLevelsString() const;
+ //! Returns the log level corresponding to a string, or nullopt if unrecognized.
+ static std::optional<BCLog::Level> GetLogLevel(std::string_view str);
//! Returns the string representation of a log level.
static std::string LogLevelToStr(BCLog::Level level);
And we could probably use this new constant in a few more places:
diff --git a/src/init/common.cpp b/src/init/common.cpp
--- a/src/init/common.cpp (revision c9516e72f9c26f76aacc510e9d39448291fc2994)
+++ b/src/init/common.cpp (revision f1467268d1159abb523fd65062d7533100a15217)
@@ -67,7 +67,7 @@
if (level_str.find_first_of(':', 3) == std::string::npos) {
// user passed a global log level, i.e. -loglevel=<level>
const auto level{BCLog::Logger::GetLogLevel(level_str)};
- if (!level || *level > BCLog::Level::Info) {
+ if (!level || *level > BCLog::UNCONDITIONAL_LOG_LEVEL) {
return util::Error{strprintf(_("Unsupported global logging level %s=%s. Valid values: %s."), "-loglevel", level_str, LogInstance().LogLevelsString())};
}
global_level = *level;
@@ -76,7 +76,7 @@
const auto& toks = SplitString(level_str, ':');
const auto category{toks.size() == 2 ? BCLog::Logger::GetLogCategory(toks[0]) : std::nullopt};
const auto level{toks.size() == 2 ? BCLog::Logger::GetLogLevel(toks[1]) : std::nullopt};
- if (!category || !level || *level > BCLog::Level::Info) {
+ if (!category || !level || *level > BCLog::UNCONDITIONAL_LOG_LEVEL) {
return util::Error{strprintf(_("Unsupported category-specific logging level %1$s=%2$s. Expected %1$s=<category>:<loglevel>. Valid categories: %3$s. Valid loglevels: %4$s."), "-loglevel", level_str, LogInstance().LogCategoriesString(), LogInstance().LogLevelsString())};
}
category_levels.emplace_back(*category, *level);
