log: clean up LogPrintStr_ and Reset, prefix all logs with "[*]" when there are suppressions
What the developer wrote
Authored by Eugene Siegel
log: clean up LogPrintStr_ and Reset, prefix all logs with "[*]" when there are suppressions
In LogPrintStr_:
- remove an unnecessary BCLog since we are in the BCLog namespace.
- remove an unnecessary \n when rate limiting is triggered since
FormatLogStrInPlace will add it.
- move the ratelimit bool into an else if block.
- prefix all log lines with [*] when suppressions exist. Previously this
was only done if should_ratelimit was true.
In Reset:
- remove an unnecessary \n since FormatLogStrInPlace will add it.
- Change Level::Info to Level::Warning.
What changed, and why it matters
This is a small cleanup change to Bitcoin Core's internal logging code. It fixes a minor bug where a '[*]' indicator that warns operators some log messages are being suppressed was only shown in one specific case, and now shows it whenever any suppression is active. It also removes a few redundant newline characters and adjusts a log message severity from 'Info' to 'Warning'. There is no direct security vulnerability here; it is a reliability/usability improvement for log output.
No security action required. Treat as a normal code-quality/logging improvement. Reviewers may want to confirm the removed '\n' characters do not affect downstream log consumers that expect explicit newlines, though FormatLogStrInPlace is documented to add them.
Security signals we found
Logging-only change with no input parsing, network, or cryptographic code modified
No change to consensus, validation, wallet, or P2P logic
Severity raised from Info to Warning for dropped-bytes notification (improved operator visibility)
Indicator prefix now correctly shown whenever log suppression is active
Evidence from the diff
The commit refactors BCLog::Logger::LogPrintStr_ and BCLog::LogRateLimiter::Reset. Key changes: (1) moves the ‘[*] ’ prefix logic outside the should_ratelimit branch so it is applied whenever SuppressionsActive() is true; (2) removes an unnecessary trailing ‘\n’ in the rate-limit warning and in Reset because FormatLogStrInPlace already appends one; (3) changes the Reset message level from Level::Info to Level::Warning; (4) moves the ratelimit bool assignment into an else-if. These are cosmetic/behavioral consistency fixes in logging, not memory-safety or consensus-critical changes.
Changed components
src/logging.cppBCLog::Logger::LogPrintStr_BCLog::LogRateLimiter::ResetInspect captured patch +12 / −10
diff --git a/src/logging.cpp b/src/logging.cpp
index befac8a0..0cad2905 100644
--- a/src/logging.cpp
+++ b/src/logging.cpp
@@ -455,24 +455,26 @@ void BCLog::Logger::LogPrintStr_(std::string_view str, std::source_location&& so
bool ratelimit{false};
if (should_ratelimit && m_limiter) {
auto status{m_limiter->Consume(source_loc, str_prefixed)};
- if (status == BCLog::LogRateLimiter::Status::NEWLY_SUPPRESSED) {
+ if (status == LogRateLimiter::Status::NEWLY_SUPPRESSED) {
// NOLINTNEXTLINE(misc-no-recursion)
LogPrintStr_(strprintf(
"Excessive logging detected from %s:%d (%s): >%d bytes logged during "
"the last time window of %is. Suppressing logging to disk from this "
"source location until time window resets. Console logging "
- "unaffected. Last log entry.\n",
+ "unaffected. Last log entry.",
source_loc.file_name(), source_loc.line(), source_loc.function_name(),
m_limiter->m_max_bytes,
Ticks<std::chrono::seconds>(m_limiter->m_reset_window)),
std::source_location::current(), LogFlags::ALL, Level::Warning, /*should_ratelimit=*/false); // with should_ratelimit=false, this cannot lead to infinite recursion
+ } else if (status == LogRateLimiter::Status::STILL_SUPPRESSED) {
+ ratelimit = true;
}
- ratelimit = status == BCLog::LogRateLimiter::Status::STILL_SUPPRESSED;
- // To avoid confusion caused by dropped log messages when debugging an issue,
- // we prefix log lines with "[*]" when there are any suppressed source locations.
- if (m_limiter->SuppressionsActive()) {
- str_prefixed.insert(0, "[*] ");
- }
+ }
+
+ // To avoid confusion caused by dropped log messages when debugging an issue,
+ // we prefix log lines with "[*]" when there are any suppressed source locations.
+ if (m_limiter && m_limiter->SuppressionsActive()) {
+ str_prefixed.insert(0, "[*] ");
}
if (m_print_to_console) {
@@ -552,8 +554,8 @@ void BCLog::LogRateLimiter::Reset()
for (const auto& [source_loc, stats] : source_locations) {
if (stats.m_dropped_bytes == 0) continue;
LogPrintLevel_(
- LogFlags::ALL, Level::Info, /*should_ratelimit=*/false,
- "Restarting logging from %s:%d (%s): %d bytes were dropped during the last %ss.\n",
+ LogFlags::ALL, Level::Warning, /*should_ratelimit=*/false,
+ "Restarting logging from %s:%d (%s): %d bytes were dropped during the last %ss.",
source_loc.file_name(), source_loc.line(), source_loc.function_name(),
stats.m_dropped_bytes, Ticks<std::chrono::seconds>(m_reset_window));
}
Why this scored 17/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.