logging: Move message formatting to util/log.h
What changed, and why it matters
This commit is a straightforward internal code reorganization for Bitcoin Core's logging system. It moves where log message formatting happens so that smaller parts of the code can include a lighter logging header. The commit itself notes a minor, non-security performance side effect in an unusual configuration, but there is no vulnerability or security fix here.
No security action required. Treat as normal refactoring; review for build/header correctness and the noted performance impact if relevant.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change relocates LogPrintFormatInternal and the detail_LogIfCategoryAndLevelEnabled macro’s category check from src/logging.h to src/util/log.h, and introduces a util::log::ShouldLog / util::log::Log abstraction backed by BCLog::Logger. The only behavioral difference noted by the author is that when Logger::WillLogCategoryLevel returns true but Logger::Enabled returns false (e.g., -noprinttoconsole -nodebuglogfile), an extra strprintf call now occurs. This is a performance/behavioral note, not a security defect.
Changed components
src/logging.cppsrc/logging.hsrc/util/log.hInspect captured patch +54 / −20
diff --git a/src/logging.cpp b/src/logging.cpp
index 1c3e30e9..df6946d6 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -602,3 +602,16 @@ bool BCLog::Logger::SetCategoryLogLevel(std::string_view category_str, std::stri
m_category_log_levels[flag] = level.value();
return true;
}
+
+bool util::log::ShouldLog(Category category, Level level)
+{
+ return LogInstance().WillLogCategoryLevel(static_cast<BCLog::LogFlags>(category), level);
+}
+
+void util::log::Log(util::log::Entry entry)
+{
+ BCLog::Logger& logger{LogInstance()};
+ if (logger.Enabled()) {
+ logger.LogPrintStr(std::move(entry.message), std::move(entry.source_loc), static_cast<BCLog::LogFlags>(entry.category), entry.level, entry.should_ratelimit);
+ }
+}
diff --git a/src/logging.h b/src/logging.h
index d41a33bc..e2149501 100644
--- a/src/logging.h
+++ b/src/logging.h
@@ -9,8 +9,6 @@
#include <crypto/siphash.h>
#include <logging/categories.h> // IWYU pragma: export
#include <threadsafety.h>
-#include <tinyformat.h>
-#include <util/check.h>
#include <util/fs.h>
#include <util/log.h> // IWYU pragma: export
#include <util/string.h>
@@ -22,11 +20,8 @@
#include <functional>
#include <list>
#include <memory>
-#include <mutex>
-#include <source_location>
#include <string>
#include <unordered_map>
-#include <unordered_set>
#include <vector>
static const bool DEFAULT_LOGTIMEMICROS = false;
@@ -304,18 +299,4 @@ static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level leve
/** Return true if str parses as a log category and set the flag */
bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);
-template <typename... Args>
-inline void LogPrintFormatInternal(SourceLocation&& source_loc, BCLog::LogFlags flag, BCLog::Level level, bool should_ratelimit, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
-{
- if (LogInstance().Enabled()) {
- std::string log_msg;
- try {
- log_msg = tfm::format(fmt, args...);
- } catch (tinyformat::format_error& fmterr) {
- log_msg = "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: " + fmt.fmt;
- }
- LogInstance().LogPrintStr(log_msg, std::move(source_loc), flag, level, should_ratelimit);
- }
-}
-
#endif // BITCOIN_LOGGING_H
diff --git a/src/util/log.h b/src/util/log.h
index ef5cbf14..aae23f0d 100644
--- a/src/util/log.h
+++ b/src/util/log.h
@@ -5,8 +5,13 @@
#ifndef BITCOIN_UTIL_LOG_H
#define BITCOIN_UTIL_LOG_H
+#include <logging/categories.h> // IWYU pragma: export
+#include <tinyformat.h>
+#include <util/check.h>
+
#include <cstdint>
#include <source_location>
+#include <string>
#include <string_view>
/// Like std::source_location, but allowing to override the function name.
@@ -30,6 +35,9 @@ private:
};
namespace util::log {
+/** Opaque to util::log; interpreted by consumers (e.g., BCLog::LogFlags). */
+using Category = uint64_t;
+
enum class Level {
Trace = 0, // High-volume or detailed logging for development/debugging
Debug, // Reasonably noisy logging, but still usable in production
@@ -37,6 +45,21 @@ enum class Level {
Warning,
Error,
};
+
+struct Entry {
+ Category category;
+ Level level;
+ bool should_ratelimit{false}; //!< Hint for consumers if this entry should be ratelimited
+ SourceLocation source_loc;
+ std::string message;
+};
+
+/** Return whether messages with specified category and level should be logged. Applications using
+ * the logging library need to provide this. */
+bool ShouldLog(Category category, Level level);
+
+/** Send message to be logged. Applications using the logging library need to provide this. */
+void Log(Entry entry);
} // namespace util::log
namespace BCLog {
@@ -44,6 +67,23 @@ namespace BCLog {
using Level = util::log::Level;
} // namespace BCLog
+template <typename... Args>
+inline void LogPrintFormatInternal(SourceLocation&& source_loc, BCLog::LogFlags flag, BCLog::Level level, bool should_ratelimit, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
+{
+ std::string log_msg;
+ try {
+ log_msg = tfm::format(fmt, args...);
+ } catch (tinyformat::format_error& fmterr) {
+ log_msg = "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: " + fmt.fmt;
+ }
+ util::log::Log(util::log::Entry{
+ .category = flag,
+ .level = level,
+ .should_ratelimit = should_ratelimit,
+ .source_loc = std::move(source_loc),
+ .message = std::move(log_msg)});
+}
+
// Allow __func__ to be used in any context without warnings:
// NOLINTNEXTLINE(bugprone-lambda-function-name)
#define LogPrintLevel_(category, level, should_ratelimit, ...) LogPrintFormatInternal(SourceLocation{__func__}, category, level, should_ratelimit, __VA_ARGS__)
@@ -64,7 +104,7 @@ using Level = util::log::Level;
// developers or power users who are aware that -debug may cause excessive disk usage due to logging.
#define detail_LogIfCategoryAndLevelEnabled(category, level, ...) \
do { \
- if (LogAcceptCategory((category), (level))) { \
+ if (util::log::ShouldLog((category), (level))) { \
bool rate_limit{level >= BCLog::Level::Info}; \
Assume(!rate_limit); /*Only called with the levels below*/ \
LogPrintLevel_(category, level, rate_limit, __VA_ARGS__); \
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.