Skip to content

Commit facd3d5

Browse files
author
MarcoFalke
committed
log: Use __func__ for -logsourcelocations
1 parent 7c7cd8c commit facd3d5

File tree

4 files changed

+53
-29
lines changed

4 files changed

+53
-29
lines changed

‎doc/release-notes-34088.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
- When `-logsourcelocations` is enabled, the log output now contains just the
2+
function name instead of the entire function signature. (#34088)

‎src/logging.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ bool BCLog::Logger::StartLogging()
7575
// dump buffered messages from before we opened the log
7676
m_buffering = false;
7777
if (m_buffer_lines_discarded > 0) {
78-
LogPrintStr_(strprintf("Early logging buffer overflowed, %d log lines discarded.\n", m_buffer_lines_discarded), std::source_location::current(), BCLog::ALL, Level::Info, /*should_ratelimit=*/false);
78+
LogPrintStr_(strprintf("Early logging buffer overflowed, %d log lines discarded.\n", m_buffer_lines_discarded), SourceLocation{__func__}, BCLog::ALL, Level::Info, /*should_ratelimit=*/false);
7979
}
8080
while (!m_msgs_before_open.empty()) {
8181
const auto& buflog = m_msgs_before_open.front();
@@ -388,7 +388,7 @@ std::shared_ptr<BCLog::LogRateLimiter> BCLog::LogRateLimiter::Create(
388388
}
389389

390390
BCLog::LogRateLimiter::Status BCLog::LogRateLimiter::Consume(
391-
const std::source_location& source_loc,
391+
const SourceLocation& source_loc,
392392
const std::string& str)
393393
{
394394
StdLockGuard scoped_lock(m_mutex);
@@ -403,14 +403,14 @@ BCLog::LogRateLimiter::Status BCLog::LogRateLimiter::Consume(
403403
return status;
404404
}
405405

406-
void BCLog::Logger::FormatLogStrInPlace(std::string& str, BCLog::LogFlags category, BCLog::Level level, const std::source_location& source_loc, std::string_view threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const
406+
void BCLog::Logger::FormatLogStrInPlace(std::string& str, BCLog::LogFlags category, BCLog::Level level, const SourceLocation& source_loc, std::string_view threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const
407407
{
408408
if (!str.ends_with('\n')) str.push_back('\n');
409409

410410
str.insert(0, GetLogPrefix(category, level));
411411

412412
if (m_log_sourcelocations) {
413-
str.insert(0, strprintf("[%s:%d] [%s] ", RemovePrefixView(source_loc.file_name(), "./"), source_loc.line(), source_loc.function_name()));
413+
str.insert(0, strprintf("[%s:%d] [%s] ", RemovePrefixView(source_loc.file_name(), "./"), source_loc.line(), source_loc.function_name_short()));
414414
}
415415

416416
if (m_log_threadnames) {
@@ -420,14 +420,14 @@ void BCLog::Logger::FormatLogStrInPlace(std::string& str, BCLog::LogFlags catego
420420
str.insert(0, LogTimestampStr(now, mocktime));
421421
}
422422

423-
void BCLog::Logger::LogPrintStr(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
423+
void BCLog::Logger::LogPrintStr(std::string_view str, SourceLocation&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
424424
{
425425
StdLockGuard scoped_lock(m_cs);
426426
return LogPrintStr_(str, std::move(source_loc), category, level, should_ratelimit);
427427
}
428428

429429
// NOLINTNEXTLINE(misc-no-recursion)
430-
void BCLog::Logger::LogPrintStr_(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
430+
void BCLog::Logger::LogPrintStr_(std::string_view str, SourceLocation&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
431431
{
432432
std::string str_prefixed = LogEscapeMessage(str);
433433

@@ -470,10 +470,10 @@ void BCLog::Logger::LogPrintStr_(std::string_view str, std::source_location&& so
470470
"the last time window of %is. Suppressing logging to disk from this "
471471
"source location until time window resets. Console logging "
472472
"unaffected. Last log entry.",
473-
source_loc.file_name(), source_loc.line(), source_loc.function_name(),
473+
source_loc.file_name(), source_loc.line(), source_loc.function_name_short(),
474474
m_limiter->m_max_bytes,
475475
Ticks<std::chrono::seconds>(m_limiter->m_reset_window)),
476-
std::source_location::current(), LogFlags::ALL, Level::Warning, /*should_ratelimit=*/false); // with should_ratelimit=false, this cannot lead to infinite recursion
476+
SourceLocation{__func__}, LogFlags::ALL, Level::Warning, /*should_ratelimit=*/false); // with should_ratelimit=false, this cannot lead to infinite recursion
477477
} else if (status == LogRateLimiter::Status::STILL_SUPPRESSED) {
478478
ratelimit = true;
479479
}
@@ -564,7 +564,7 @@ void BCLog::LogRateLimiter::Reset()
564564
LogPrintLevel_(
565565
LogFlags::ALL, Level::Warning, /*should_ratelimit=*/false,
566566
"Restarting logging from %s:%d (%s): %d bytes were dropped during the last %ss.",
567-
source_loc.file_name(), source_loc.line(), source_loc.function_name(),
567+
source_loc.file_name(), source_loc.line(), source_loc.function_name_short(),
568568
stats.m_dropped_bytes, Ticks<std::chrono::seconds>(m_reset_window));
569569
}
570570
}

‎src/logging.h‎

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,35 @@ extern const char * const DEFAULT_DEBUGLOGFILE;
3737

3838
extern bool fLogIPs;
3939

40+
/// Like std::source_location, but allowing to override the function name.
41+
class SourceLocation
42+
{
43+
public:
44+
/// The func argument must be constructed from the C++11 __func__ macro.
45+
/// Ref: https://en.cppreference.com/w/cpp/language/function.html#func
46+
/// Non-static string literals are not supported.
47+
SourceLocation(const char* func,
48+
std::source_location loc = std::source_location::current())
49+
: m_func{func}, m_loc{loc} {}
50+
51+
std::string_view file_name() const { return m_loc.file_name(); }
52+
std::uint_least32_t line() const { return m_loc.line(); }
53+
std::string_view function_name_short() const { return m_func; }
54+
55+
private:
56+
std::string_view m_func;
57+
std::source_location m_loc;
58+
};
59+
4060
struct SourceLocationEqual {
41-
bool operator()(const std::source_location& lhs, const std::source_location& rhs) const noexcept
61+
bool operator()(const SourceLocation& lhs, const SourceLocation& rhs) const noexcept
4262
{
4363
return lhs.line() == rhs.line() && std::string_view(lhs.file_name()) == std::string_view(rhs.file_name());
4464
}
4565
};
4666

4767
struct SourceLocationHasher {
48-
size_t operator()(const std::source_location& s) const noexcept
68+
size_t operator()(const SourceLocation& s) const noexcept
4969
{
5070
// Use CSipHasher(0, 0) as a simple way to get uniform distribution.
5171
return size_t(CSipHasher(0, 0)
@@ -131,7 +151,7 @@ namespace BCLog {
131151
mutable StdMutex m_mutex;
132152

133153
//! Stats for each source location that has attempted to log something.
134-
std::unordered_map<std::source_location, Stats, SourceLocationHasher, SourceLocationEqual> m_source_locations GUARDED_BY(m_mutex);
154+
std::unordered_map<SourceLocation, Stats, SourceLocationHasher, SourceLocationEqual> m_source_locations GUARDED_BY(m_mutex);
135155
//! Whether any log locations are suppressed. Cached view on m_source_locations for performance reasons.
136156
std::atomic<bool> m_suppression_active{false};
137157
LogRateLimiter(uint64_t max_bytes, std::chrono::seconds reset_window);
@@ -163,7 +183,7 @@ namespace BCLog {
163183
//! Consumes `source_loc`'s available bytes corresponding to the size of the (formatted)
164184
//! `str` and returns its status.
165185
[[nodiscard]] Status Consume(
166-
const std::source_location& source_loc,
186+
const SourceLocation& source_loc,
167187
const std::string& str) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
168188
//! Resets all usage to zero. Called periodically by the scheduler.
169189
void Reset() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);
@@ -178,7 +198,7 @@ namespace BCLog {
178198
SystemClock::time_point now;
179199
std::chrono::seconds mocktime;
180200
std::string str, threadname;
181-
std::source_location source_loc;
201+
SourceLocation source_loc;
182202
LogFlags category;
183203
Level level;
184204
};
@@ -206,15 +226,15 @@ namespace BCLog {
206226
/** Log categories bitfield. */
207227
std::atomic<CategoryMask> m_categories{BCLog::NONE};
208228

209-
void FormatLogStrInPlace(std::string& str, LogFlags category, Level level, const std::source_location& source_loc, std::string_view threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const;
229+
void FormatLogStrInPlace(std::string& str, LogFlags category, Level level, const SourceLocation& source_loc, std::string_view threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const;
210230

211231
std::string LogTimestampStr(SystemClock::time_point now, std::chrono::seconds mocktime) const;
212232

213233
/** Slots that connect to the print signal */
214234
std::list<std::function<void(const std::string&)>> m_print_callbacks GUARDED_BY(m_cs) {};
215235

216236
/** Send a string to the log output (internal) */
217-
void LogPrintStr_(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
237+
void LogPrintStr_(std::string_view str, SourceLocation&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
218238
EXCLUSIVE_LOCKS_REQUIRED(m_cs);
219239

220240
std::string GetLogPrefix(LogFlags category, Level level) const;
@@ -233,7 +253,7 @@ namespace BCLog {
233253
std::atomic<bool> m_reopen_file{false};
234254

235255
/** Send a string to the log output */
236-
void LogPrintStr(std::string_view str, std::source_location&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
256+
void LogPrintStr(std::string_view str, SourceLocation&& source_loc, BCLog::LogFlags category, BCLog::Level level, bool should_ratelimit)
237257
EXCLUSIVE_LOCKS_REQUIRED(!m_cs);
238258

239259
/** Returns whether logs will be written to any output */
@@ -347,7 +367,7 @@ static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level leve
347367
bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);
348368

349369
template <typename... Args>
350-
inline void LogPrintFormatInternal(std::source_location&& source_loc, BCLog::LogFlags flag, BCLog::Level level, bool should_ratelimit, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
370+
inline void LogPrintFormatInternal(SourceLocation&& source_loc, BCLog::LogFlags flag, BCLog::Level level, bool should_ratelimit, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)
351371
{
352372
if (LogInstance().Enabled()) {
353373
std::string log_msg;
@@ -360,7 +380,9 @@ inline void LogPrintFormatInternal(std::source_location&& source_loc, BCLog::Log
360380
}
361381
}
362382

363-
#define LogPrintLevel_(category, level, should_ratelimit, ...) LogPrintFormatInternal(std::source_location::current(), category, level, should_ratelimit, __VA_ARGS__)
383+
// Allow __func__ to be used in any context without warnings:
384+
// NOLINTNEXTLINE(bugprone-lambda-function-name)
385+
#define LogPrintLevel_(category, level, should_ratelimit, ...) LogPrintFormatInternal(SourceLocation{__func__}, category, level, should_ratelimit, __VA_ARGS__)
364386

365387
// Log unconditionally. Uses basic rate limiting to mitigate disk filling attacks.
366388
// Be conservative when using functions that unconditionally log to debug.log!

‎src/test/logging_tests.cpp‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -118,21 +118,21 @@ BOOST_FIXTURE_TEST_CASE(logging_LogPrintStr, LogSetup)
118118
BCLog::LogFlags category;
119119
BCLog::Level level;
120120
std::string prefix;
121-
std::source_location loc;
121+
SourceLocation loc;
122122
};
123123

124124
std::vector<Case> cases = {
125-
{"foo1: bar1", BCLog::NET, BCLog::Level::Debug, "[net] ", std::source_location::current()},
126-
{"foo2: bar2", BCLog::NET, BCLog::Level::Info, "[net:info] ", std::source_location::current()},
127-
{"foo3: bar3", BCLog::ALL, BCLog::Level::Debug, "[debug] ", std::source_location::current()},
128-
{"foo4: bar4", BCLog::ALL, BCLog::Level::Info, "", std::source_location::current()},
129-
{"foo5: bar5", BCLog::NONE, BCLog::Level::Debug, "[debug] ", std::source_location::current()},
130-
{"foo6: bar6", BCLog::NONE, BCLog::Level::Info, "", std::source_location::current()},
125+
{"foo1: bar1", BCLog::NET, BCLog::Level::Debug, "[net] ", SourceLocation{__func__}},
126+
{"foo2: bar2", BCLog::NET, BCLog::Level::Info, "[net:info] ", SourceLocation{__func__}},
127+
{"foo3: bar3", BCLog::ALL, BCLog::Level::Debug, "[debug] ", SourceLocation{__func__}},
128+
{"foo4: bar4", BCLog::ALL, BCLog::Level::Info, "", SourceLocation{__func__}},
129+
{"foo5: bar5", BCLog::NONE, BCLog::Level::Debug, "[debug] ", SourceLocation{__func__}},
130+
{"foo6: bar6", BCLog::NONE, BCLog::Level::Info, "", SourceLocation{__func__}},
131131
};
132132

133133
std::vector<std::string> expected;
134134
for (auto& [msg, category, level, prefix, loc] : cases) {
135-
expected.push_back(tfm::format("[%s:%s] [%s] %s%s", util::RemovePrefix(loc.file_name(), "./"), loc.line(), loc.function_name(), prefix, msg));
135+
expected.push_back(tfm::format("[%s:%s] [%s] %s%s", util::RemovePrefix(loc.file_name(), "./"), loc.line(), loc.function_name_short(), prefix, msg));
136136
LogInstance().LogPrintStr(msg, std::move(loc), category, level, /*should_ratelimit=*/false);
137137
}
138138
std::vector<std::string> log_lines{ReadDebugLogLines()};
@@ -306,8 +306,8 @@ BOOST_AUTO_TEST_CASE(logging_log_rate_limiter)
306306
auto& limiter{*Assert(limiter_)};
307307

308308
using Status = BCLog::LogRateLimiter::Status;
309-
auto source_loc_1{std::source_location::current()};
310-
auto source_loc_2{std::source_location::current()};
309+
auto source_loc_1{SourceLocation{__func__}};
310+
auto source_loc_2{SourceLocation{__func__}};
311311

312312
// A fresh limiter should not have any suppressions
313313
BOOST_CHECK(!limiter.SuppressionsActive());

0 commit comments

Comments
 (0)