From 6bbc2dd6c50f09ff1e70423dc29a404b570f5b69 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Wed, 3 Jul 2024 23:46:32 +1000 Subject: [PATCH 1/5] logging: Add thread safety annotations --- src/logging.h | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/logging.h b/src/logging.h index fe6b7051baf..2f24f328861 100644 --- a/src/logging.h +++ b/src/logging.h @@ -127,17 +127,18 @@ namespace BCLog { std::string GetLogPrefix(LogFlags category, Level level) const; /** Send a string to the log output */ - void LogPrintStr(const std::string& str, const std::string& logging_function, const std::string& source_file, int source_line, BCLog::LogFlags category, BCLog::Level level); + void LogPrintStr(const std::string& str, const std::string& logging_function, const std::string& source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) + EXCLUSIVE_LOCKS_REQUIRED(!m_cs); /** Returns whether logs will be written to any output */ - bool Enabled() const + bool Enabled() const EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { StdLockGuard scoped_lock(m_cs); return m_buffering || m_print_to_console || m_print_to_file || !m_print_callbacks.empty(); } /** Connect a slot to the print signal and return the connection */ - std::list>::iterator PushBackCallback(std::function fun) + std::list>::iterator PushBackCallback(std::function fun) EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { StdLockGuard scoped_lock(m_cs); m_print_callbacks.push_back(std::move(fun)); @@ -145,30 +146,30 @@ namespace BCLog { } /** Delete a connection */ - void DeleteCallback(std::list>::iterator it) + void DeleteCallback(std::list>::iterator it) EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { StdLockGuard scoped_lock(m_cs); m_print_callbacks.erase(it); } /** Start logging (and flush all buffered messages) */ - bool StartLogging(); + bool StartLogging() EXCLUSIVE_LOCKS_REQUIRED(!m_cs); /** Only for testing */ - void DisconnectTestLogger(); + void DisconnectTestLogger() EXCLUSIVE_LOCKS_REQUIRED(!m_cs); void ShrinkDebugFile(); - std::unordered_map CategoryLevels() const + std::unordered_map CategoryLevels() const EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { StdLockGuard scoped_lock(m_cs); return m_category_log_levels; } - void SetCategoryLogLevel(const std::unordered_map& levels) + void SetCategoryLogLevel(const std::unordered_map& levels) EXCLUSIVE_LOCKS_REQUIRED(!m_cs) { StdLockGuard scoped_lock(m_cs); m_category_log_levels = levels; } - bool SetCategoryLogLevel(const std::string& category_str, const std::string& level_str); + bool SetCategoryLogLevel(const std::string& category_str, const std::string& level_str) EXCLUSIVE_LOCKS_REQUIRED(!m_cs); Level LogLevel() const { return m_log_level.load(); } void SetLogLevel(Level level) { m_log_level = level; } @@ -182,7 +183,7 @@ namespace BCLog { bool DisableCategory(const std::string& str); bool WillLogCategory(LogFlags category) const; - bool WillLogCategoryLevel(LogFlags category, Level level) const; + bool WillLogCategoryLevel(LogFlags category, Level level) const EXCLUSIVE_LOCKS_REQUIRED(!m_cs); /** Returns a vector of the log categories in alphabetical order. */ std::vector LogCategoriesList() const; From 0b1960f1b29cfe5209ac68102c8643fc9553f247 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Thu, 4 Jul 2024 00:52:29 +1000 Subject: [PATCH 2/5] logging: Add DisableLogging() --- src/bitcoin-chainstate.cpp | 7 +++++++ src/logging.cpp | 12 ++++++++++++ src/logging.h | 8 ++++++++ 3 files changed, 27 insertions(+) diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index ecbdcd48bb3..697096586e5 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -42,6 +43,12 @@ int main(int argc, char* argv[]) { + // We do not enable logging for this app, so explicitly disable it. + // To enable logging instead, replace with: + // LogInstance().m_print_to_console = true; + // LogInstance().StartLogging(); + LogInstance().DisableLogging(); + // SETUP: Argument parsing and handling if (argc != 2) { std::cerr diff --git a/src/logging.cpp b/src/logging.cpp index a9fea433be4..53af7d5ca72 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -96,6 +96,18 @@ void BCLog::Logger::DisconnectTestLogger() m_print_callbacks.clear(); } +void BCLog::Logger::DisableLogging() +{ + { + StdLockGuard scoped_lock(m_cs); + assert(m_buffering); + assert(m_print_callbacks.empty()); + } + m_print_to_file = false; + m_print_to_console = false; + StartLogging(); +} + void BCLog::Logger::EnableCategory(BCLog::LogFlags flag) { m_categories |= flag; diff --git a/src/logging.h b/src/logging.h index 2f24f328861..70539f03b05 100644 --- a/src/logging.h +++ b/src/logging.h @@ -157,6 +157,14 @@ namespace BCLog { /** Only for testing */ void DisconnectTestLogger() EXCLUSIVE_LOCKS_REQUIRED(!m_cs); + /** Disable logging + * This offers a slight speedup and slightly smaller memory usage + * compared to leaving the logging system in its default state. + * Mostly intended for libbitcoin-kernel apps that don't want any logging. + * Should be used instead of StartLogging(). + */ + void DisableLogging() EXCLUSIVE_LOCKS_REQUIRED(!m_cs); + void ShrinkDebugFile(); std::unordered_map CategoryLevels() const EXCLUSIVE_LOCKS_REQUIRED(!m_cs) From 6cf9b344409efcc41a2b34f87100d25e1d2486af Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Wed, 3 Jul 2024 23:51:32 +1000 Subject: [PATCH 3/5] logging: Limit early logging buffer Log messages created prior to StartLogging() being called go into a buffer. Enforce a limit on the size of this buffer. --- src/logging.cpp | 31 +++++++++++++++++++++++++++++++ src/logging.h | 8 ++++++++ 2 files changed, 39 insertions(+) diff --git a/src/logging.cpp b/src/logging.cpp index 53af7d5ca72..9dafa720544 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -4,6 +4,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include +#include #include #include #include @@ -71,6 +72,9 @@ bool BCLog::Logger::StartLogging() // dump buffered messages from before we opened the log m_buffering = false; + if (m_buffer_lines_discarded > 0) { + LogPrintStr_(strprintf("Early logging buffer overflowed, %d log lines discarded.\n", m_buffer_lines_discarded), __func__, __FILE__, __LINE__, BCLog::ALL, Level::Info); + } while (!m_msgs_before_open.empty()) { const std::string& s = m_msgs_before_open.front(); @@ -82,6 +86,7 @@ bool BCLog::Logger::StartLogging() m_msgs_before_open.pop_front(); } + m_cur_buffer_memusage = 0; if (m_print_to_console) fflush(stdout); return true; @@ -94,6 +99,11 @@ void BCLog::Logger::DisconnectTestLogger() if (m_fileout != nullptr) fclose(m_fileout); m_fileout = nullptr; m_print_callbacks.clear(); + m_max_buffer_memusage = DEFAULT_MAX_LOG_BUFFER; + m_cur_buffer_memusage = 0; + m_buffer_lines_discarded = 0; + m_msgs_before_open.clear(); + } void BCLog::Logger::DisableLogging() @@ -362,9 +372,19 @@ std::string BCLog::Logger::GetLogPrefix(BCLog::LogFlags category, BCLog::Level l return s; } +static size_t MemUsage(const std::string& str) +{ + return str.size() + memusage::MallocUsage(sizeof(memusage::list_node)); +} + void BCLog::Logger::LogPrintStr(const std::string& str, const std::string& logging_function, const std::string& source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) { StdLockGuard scoped_lock(m_cs); + return LogPrintStr_(str, logging_function, source_file, source_line, category, level); +} + +void BCLog::Logger::LogPrintStr_(const std::string& str, const std::string& logging_function, const std::string& source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) +{ std::string str_prefixed = LogEscapeMessage(str); if (m_started_new_line) { @@ -387,6 +407,17 @@ void BCLog::Logger::LogPrintStr(const std::string& str, const std::string& loggi if (m_buffering) { // buffer if we haven't started logging yet m_msgs_before_open.push_back(str_prefixed); + + m_cur_buffer_memusage += MemUsage(str_prefixed); + while (m_cur_buffer_memusage > m_max_buffer_memusage) { + if (m_msgs_before_open.empty()) { + m_cur_buffer_memusage = 0; + break; + } + m_cur_buffer_memusage -= MemUsage(m_msgs_before_open.front()); + m_msgs_before_open.pop_front(); + ++m_buffer_lines_discarded; + } return; } diff --git a/src/logging.h b/src/logging.h index 70539f03b05..b3fe70cca70 100644 --- a/src/logging.h +++ b/src/logging.h @@ -79,6 +79,7 @@ namespace BCLog { Error, }; constexpr auto DEFAULT_LOG_LEVEL{Level::Debug}; + constexpr size_t DEFAULT_MAX_LOG_BUFFER{1'000'000}; // buffer up to 1MB of log data prior to StartLogging class Logger { @@ -88,6 +89,9 @@ namespace BCLog { FILE* m_fileout GUARDED_BY(m_cs) = nullptr; std::list m_msgs_before_open GUARDED_BY(m_cs); bool m_buffering GUARDED_BY(m_cs) = true; //!< Buffer messages before logging can be started. + size_t m_max_buffer_memusage GUARDED_BY(m_cs){DEFAULT_MAX_LOG_BUFFER}; + size_t m_cur_buffer_memusage GUARDED_BY(m_cs){0}; + size_t m_buffer_lines_discarded GUARDED_BY(m_cs){0}; /** * m_started_new_line is a state variable that will suppress printing of @@ -111,6 +115,10 @@ namespace BCLog { /** Slots that connect to the print signal */ std::list> m_print_callbacks GUARDED_BY(m_cs) {}; + /** Send a string to the log output (internal) */ + void LogPrintStr_(const std::string& str, const std::string& logging_function, const std::string& source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) + EXCLUSIVE_LOCKS_REQUIRED(m_cs); + public: bool m_print_to_console = false; bool m_print_to_file = false; From 558df5c733d31456faf856d44f7037f41981d797 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Fri, 12 Jul 2024 10:19:55 +1000 Subject: [PATCH 4/5] logging: Apply formatting to early log messages The formatting of log messages isn't defined until StartLogging() is called; so can't be correctly applied to early log messages from prior to that call. Instead of saving the output log message, save the inputs to the logging invocation (including time, mocktime and thread name), and format those inputs into a log message when StartLogging() is called. --- src/logging.cpp | 105 ++++++++++++++++++++++++++++++------------------ src/logging.h | 17 +++++++- 2 files changed, 80 insertions(+), 42 deletions(-) diff --git a/src/logging.cpp b/src/logging.cpp index 9dafa720544..4b3b58a0b74 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -76,15 +76,16 @@ bool BCLog::Logger::StartLogging() LogPrintStr_(strprintf("Early logging buffer overflowed, %d log lines discarded.\n", m_buffer_lines_discarded), __func__, __FILE__, __LINE__, BCLog::ALL, Level::Info); } while (!m_msgs_before_open.empty()) { - const std::string& s = m_msgs_before_open.front(); + const auto& buflog = m_msgs_before_open.front(); + std::string s{buflog.str}; + FormatLogStrInPlace(s, buflog.category, buflog.level, buflog.source_file, buflog.source_line, buflog.logging_function, buflog.threadname, buflog.now, buflog.mocktime); + m_msgs_before_open.pop_front(); if (m_print_to_file) FileWriteStr(s, m_fileout); if (m_print_to_console) fwrite(s.data(), 1, s.size(), stdout); for (const auto& cb : m_print_callbacks) { cb(s); } - - m_msgs_before_open.pop_front(); } m_cur_buffer_memusage = 0; if (m_print_to_console) fflush(stdout); @@ -298,28 +299,23 @@ std::string BCLog::Logger::LogLevelsString() const return Join(std::vector{levels.begin(), levels.end()}, ", ", [](BCLog::Level level) { return LogLevelToStr(level); }); } -std::string BCLog::Logger::LogTimestampStr(const std::string& str) +std::string BCLog::Logger::LogTimestampStr(SystemClock::time_point now, std::chrono::seconds mocktime) const { std::string strStamped; if (!m_log_timestamps) - return str; + return strStamped; - if (m_started_new_line) { - const auto now{SystemClock::now()}; - const auto now_seconds{std::chrono::time_point_cast(now)}; - strStamped = FormatISO8601DateTime(TicksSinceEpoch(now_seconds)); - if (m_log_time_micros && !strStamped.empty()) { - strStamped.pop_back(); - strStamped += strprintf(".%06dZ", Ticks(now - now_seconds)); - } - std::chrono::seconds mocktime = GetMockTime(); - if (mocktime > 0s) { - strStamped += " (mocktime: " + FormatISO8601DateTime(count_seconds(mocktime)) + ")"; - } - strStamped += ' ' + str; - } else - strStamped = str; + const auto now_seconds{std::chrono::time_point_cast(now)}; + strStamped = FormatISO8601DateTime(TicksSinceEpoch(now_seconds)); + if (m_log_time_micros && !strStamped.empty()) { + strStamped.pop_back(); + strStamped += strprintf(".%06dZ", Ticks(now - now_seconds)); + } + if (mocktime > 0s) { + strStamped += " (mocktime: " + FormatISO8601DateTime(count_seconds(mocktime)) + ")"; + } + strStamped += ' '; return strStamped; } @@ -372,9 +368,24 @@ std::string BCLog::Logger::GetLogPrefix(BCLog::LogFlags category, BCLog::Level l return s; } -static size_t MemUsage(const std::string& str) +static size_t MemUsage(const BCLog::Logger::BufferedLog& buflog) { - return str.size() + memusage::MallocUsage(sizeof(memusage::list_node)); + return buflog.str.size() + buflog.logging_function.size() + buflog.source_file.size() + buflog.threadname.size() + memusage::MallocUsage(sizeof(memusage::list_node)); +} + +void BCLog::Logger::FormatLogStrInPlace(std::string& str, BCLog::LogFlags category, BCLog::Level level, const std::string& source_file, int source_line, const std::string& logging_function, const std::string& threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const +{ + str.insert(0, GetLogPrefix(category, level)); + + if (m_log_sourcelocations) { + str.insert(0, "[" + RemovePrefix(source_file, "./") + ":" + ToString(source_line) + "] [" + logging_function + "] "); + } + + if (m_log_threadnames) { + str.insert(0, "[" + (threadname.empty() ? "unknown" : threadname) + "] "); + } + + str.insert(0, LogTimestampStr(now, mocktime)); } void BCLog::Logger::LogPrintStr(const std::string& str, const std::string& logging_function, const std::string& source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) @@ -387,28 +398,37 @@ void BCLog::Logger::LogPrintStr_(const std::string& str, const std::string& logg { std::string str_prefixed = LogEscapeMessage(str); - if (m_started_new_line) { - str_prefixed.insert(0, GetLogPrefix(category, level)); - } - - if (m_log_sourcelocations && m_started_new_line) { - str_prefixed.insert(0, "[" + RemovePrefix(source_file, "./") + ":" + ToString(source_line) + "] [" + logging_function + "] "); - } - - if (m_log_threadnames && m_started_new_line) { - const auto& threadname = util::ThreadGetInternalName(); - str_prefixed.insert(0, "[" + (threadname.empty() ? "unknown" : threadname) + "] "); - } - - str_prefixed = LogTimestampStr(str_prefixed); - + const bool starts_new_line = m_started_new_line; m_started_new_line = !str.empty() && str[str.size()-1] == '\n'; if (m_buffering) { - // buffer if we haven't started logging yet - m_msgs_before_open.push_back(str_prefixed); + if (!starts_new_line) { + if (!m_msgs_before_open.empty()) { + m_msgs_before_open.back().str += str_prefixed; + m_cur_buffer_memusage += str_prefixed.size(); + return; + } else { + // unlikely edge case; add a marker that something was trimmed + str_prefixed.insert(0, "[...] "); + } + } + + { + BufferedLog buf{ + .now=SystemClock::now(), + .mocktime=GetMockTime(), + .str=str_prefixed, + .logging_function=logging_function, + .source_file=source_file, + .threadname=util::ThreadGetInternalName(), + .source_line=source_line, + .category=category, + .level=level, + }; + m_cur_buffer_memusage += MemUsage(buf); + m_msgs_before_open.push_back(std::move(buf)); + } - m_cur_buffer_memusage += MemUsage(str_prefixed); while (m_cur_buffer_memusage > m_max_buffer_memusage) { if (m_msgs_before_open.empty()) { m_cur_buffer_memusage = 0; @@ -418,9 +438,14 @@ void BCLog::Logger::LogPrintStr_(const std::string& str, const std::string& logg m_msgs_before_open.pop_front(); ++m_buffer_lines_discarded; } + return; } + if (starts_new_line) { + FormatLogStrInPlace(str_prefixed, category, level, source_file, source_line, logging_function, util::ThreadGetInternalName(), SystemClock::now(), GetMockTime()); + } + if (m_print_to_console) { // print to console fwrite(str_prefixed.data(), 1, str_prefixed.size(), stdout); diff --git a/src/logging.h b/src/logging.h index b3fe70cca70..5c64c9e447e 100644 --- a/src/logging.h +++ b/src/logging.h @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -83,11 +84,21 @@ namespace BCLog { class Logger { + public: + struct BufferedLog { + SystemClock::time_point now; + std::chrono::seconds mocktime; + std::string str, logging_function, source_file, threadname; + int source_line; + LogFlags category; + Level level; + }; + private: mutable StdMutex m_cs; // Can not use Mutex from sync.h because in debug mode it would cause a deadlock when a potential deadlock was detected FILE* m_fileout GUARDED_BY(m_cs) = nullptr; - std::list m_msgs_before_open GUARDED_BY(m_cs); + std::list m_msgs_before_open GUARDED_BY(m_cs); bool m_buffering GUARDED_BY(m_cs) = true; //!< Buffer messages before logging can be started. size_t m_max_buffer_memusage GUARDED_BY(m_cs){DEFAULT_MAX_LOG_BUFFER}; size_t m_cur_buffer_memusage GUARDED_BY(m_cs){0}; @@ -110,7 +121,9 @@ namespace BCLog { /** Log categories bitfield. */ std::atomic m_categories{0}; - std::string LogTimestampStr(const std::string& str); + void FormatLogStrInPlace(std::string& str, LogFlags category, Level level, const std::string& source_file, int source_line, const std::string& logging_function, const std::string& threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const; + + std::string LogTimestampStr(SystemClock::time_point now, std::chrono::seconds mocktime) const; /** Slots that connect to the print signal */ std::list> m_print_callbacks GUARDED_BY(m_cs) {}; From b4dd7ab43e8cfc2c171f67588e4e1ec2705393c2 Mon Sep 17 00:00:00 2001 From: Anthony Towns Date: Fri, 12 Jul 2024 12:40:05 +1000 Subject: [PATCH 5/5] logging: use std::string_view --- src/logging.cpp | 37 ++++++++++++++++++------------------- src/logging.h | 18 +++++++++--------- src/test/util_tests.cpp | 2 +- 3 files changed, 28 insertions(+), 29 deletions(-) diff --git a/src/logging.cpp b/src/logging.cpp index 4b3b58a0b74..9c87cfd2b71 100644 --- a/src/logging.cpp +++ b/src/logging.cpp @@ -15,8 +15,7 @@ #include using util::Join; -using util::RemovePrefix; -using util::ToString; +using util::RemovePrefixView; const char * const DEFAULT_DEBUGLOGFILE = "debug.log"; constexpr auto MAX_USER_SETABLE_SEVERITY_LEVEL{BCLog::Level::Info}; @@ -44,7 +43,7 @@ BCLog::Logger& LogInstance() bool fLogIPs = DEFAULT_LOGIPS; -static int FileWriteStr(const std::string &str, FILE *fp) +static int FileWriteStr(std::string_view str, FILE *fp) { return fwrite(str.data(), 1, str.size(), fp); } @@ -124,7 +123,7 @@ void BCLog::Logger::EnableCategory(BCLog::LogFlags flag) m_categories |= flag; } -bool BCLog::Logger::EnableCategory(const std::string& str) +bool BCLog::Logger::EnableCategory(std::string_view str) { BCLog::LogFlags flag; if (!GetLogCategory(flag, str)) return false; @@ -137,7 +136,7 @@ void BCLog::Logger::DisableCategory(BCLog::LogFlags flag) m_categories &= ~flag; } -bool BCLog::Logger::DisableCategory(const std::string& str) +bool BCLog::Logger::DisableCategory(std::string_view str) { BCLog::LogFlags flag; if (!GetLogCategory(flag, str)) return false; @@ -168,7 +167,7 @@ bool BCLog::Logger::DefaultShrinkDebugFile() const return m_categories == BCLog::NONE; } -static const std::map LOG_CATEGORIES_BY_STR{ +static const std::map> LOG_CATEGORIES_BY_STR{ {"0", BCLog::NONE}, {"", BCLog::NONE}, {"net", BCLog::NET}, @@ -208,7 +207,7 @@ static const std::map LOG_CATEGORIES_BY_STR{ static const std::unordered_map LOG_CATEGORIES_BY_FLAG{ // Swap keys and values from LOG_CATEGORIES_BY_STR. - [](const std::map& in) { + [](const auto& in) { std::unordered_map out; for (const auto& [k, v] : in) { switch (v) { @@ -221,7 +220,7 @@ static const std::unordered_map LOG_CATEGORIES_BY_ }(LOG_CATEGORIES_BY_STR) }; -bool GetLogCategory(BCLog::LogFlags& flag, const std::string& str) +bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str) { if (str.empty()) { flag = BCLog::ALL; @@ -259,7 +258,7 @@ std::string LogCategoryToStr(BCLog::LogFlags category) return it->second; } -static std::optional GetLogLevel(const std::string& level_str) +static std::optional GetLogLevel(std::string_view level_str) { if (level_str == "trace") { return BCLog::Level::Trace; @@ -328,7 +327,7 @@ namespace BCLog { * It escapes instead of removes them to still allow for troubleshooting * issues where they accidentally end up in strings. */ - std::string LogEscapeMessage(const std::string& str) { + std::string LogEscapeMessage(std::string_view str) { std::string ret; for (char ch_in : str) { uint8_t ch = (uint8_t)ch_in; @@ -373,28 +372,28 @@ static size_t MemUsage(const BCLog::Logger::BufferedLog& buflog) return buflog.str.size() + buflog.logging_function.size() + buflog.source_file.size() + buflog.threadname.size() + memusage::MallocUsage(sizeof(memusage::list_node)); } -void BCLog::Logger::FormatLogStrInPlace(std::string& str, BCLog::LogFlags category, BCLog::Level level, const std::string& source_file, int source_line, const std::string& logging_function, const std::string& threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const +void BCLog::Logger::FormatLogStrInPlace(std::string& str, BCLog::LogFlags category, BCLog::Level level, std::string_view source_file, int source_line, std::string_view logging_function, std::string_view threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const { str.insert(0, GetLogPrefix(category, level)); if (m_log_sourcelocations) { - str.insert(0, "[" + RemovePrefix(source_file, "./") + ":" + ToString(source_line) + "] [" + logging_function + "] "); + str.insert(0, strprintf("[%s:%d] [%s] ", RemovePrefixView(source_file, "./"), source_line, logging_function)); } if (m_log_threadnames) { - str.insert(0, "[" + (threadname.empty() ? "unknown" : threadname) + "] "); + str.insert(0, strprintf("[%s] ", (threadname.empty() ? "unknown" : threadname))); } str.insert(0, LogTimestampStr(now, mocktime)); } -void BCLog::Logger::LogPrintStr(const std::string& str, const std::string& logging_function, const std::string& source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) +void BCLog::Logger::LogPrintStr(std::string_view str, std::string_view logging_function, std::string_view source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) { StdLockGuard scoped_lock(m_cs); return LogPrintStr_(str, logging_function, source_file, source_line, category, level); } -void BCLog::Logger::LogPrintStr_(const std::string& str, const std::string& logging_function, const std::string& source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) +void BCLog::Logger::LogPrintStr_(std::string_view str, std::string_view logging_function, std::string_view source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) { std::string str_prefixed = LogEscapeMessage(str); @@ -418,8 +417,8 @@ void BCLog::Logger::LogPrintStr_(const std::string& str, const std::string& logg .now=SystemClock::now(), .mocktime=GetMockTime(), .str=str_prefixed, - .logging_function=logging_function, - .source_file=source_file, + .logging_function=std::string(logging_function), + .source_file=std::string(source_file), .threadname=util::ThreadGetInternalName(), .source_line=source_line, .category=category, @@ -512,7 +511,7 @@ void BCLog::Logger::ShrinkDebugFile() fclose(file); } -bool BCLog::Logger::SetLogLevel(const std::string& level_str) +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; @@ -520,7 +519,7 @@ bool BCLog::Logger::SetLogLevel(const std::string& level_str) return true; } -bool BCLog::Logger::SetCategoryLogLevel(const std::string& category_str, const std::string& level_str) +bool BCLog::Logger::SetCategoryLogLevel(std::string_view category_str, std::string_view level_str) { BCLog::LogFlags flag; if (!GetLogCategory(flag, category_str)) return false; diff --git a/src/logging.h b/src/logging.h index 5c64c9e447e..91f919e8223 100644 --- a/src/logging.h +++ b/src/logging.h @@ -121,7 +121,7 @@ namespace BCLog { /** Log categories bitfield. */ std::atomic m_categories{0}; - void FormatLogStrInPlace(std::string& str, LogFlags category, Level level, const std::string& source_file, int source_line, const std::string& logging_function, const std::string& threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const; + void FormatLogStrInPlace(std::string& str, LogFlags category, Level level, std::string_view source_file, int source_line, std::string_view logging_function, std::string_view threadname, SystemClock::time_point now, std::chrono::seconds mocktime) const; std::string LogTimestampStr(SystemClock::time_point now, std::chrono::seconds mocktime) const; @@ -129,7 +129,7 @@ namespace BCLog { std::list> m_print_callbacks GUARDED_BY(m_cs) {}; /** Send a string to the log output (internal) */ - void LogPrintStr_(const std::string& str, const std::string& logging_function, const std::string& source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) + void LogPrintStr_(std::string_view str, std::string_view logging_function, std::string_view source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) EXCLUSIVE_LOCKS_REQUIRED(m_cs); public: @@ -148,7 +148,7 @@ namespace BCLog { std::string GetLogPrefix(LogFlags category, Level level) const; /** Send a string to the log output */ - void LogPrintStr(const std::string& str, const std::string& logging_function, const std::string& source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) + void LogPrintStr(std::string_view str, std::string_view logging_function, std::string_view source_file, int source_line, BCLog::LogFlags category, BCLog::Level level) EXCLUSIVE_LOCKS_REQUIRED(!m_cs); /** Returns whether logs will be written to any output */ @@ -198,18 +198,18 @@ namespace BCLog { StdLockGuard scoped_lock(m_cs); m_category_log_levels = levels; } - bool SetCategoryLogLevel(const std::string& category_str, const std::string& level_str) EXCLUSIVE_LOCKS_REQUIRED(!m_cs); + bool SetCategoryLogLevel(std::string_view category_str, std::string_view level_str) EXCLUSIVE_LOCKS_REQUIRED(!m_cs); Level LogLevel() const { return m_log_level.load(); } void SetLogLevel(Level level) { m_log_level = level; } - bool SetLogLevel(const std::string& level); + bool SetLogLevel(std::string_view level); uint32_t GetCategoryMask() const { return m_categories.load(); } void EnableCategory(LogFlags flag); - bool EnableCategory(const std::string& str); + bool EnableCategory(std::string_view str); void DisableCategory(LogFlags flag); - bool DisableCategory(const std::string& str); + bool DisableCategory(std::string_view str); bool WillLogCategory(LogFlags category) const; bool WillLogCategoryLevel(LogFlags category, Level level) const EXCLUSIVE_LOCKS_REQUIRED(!m_cs); @@ -242,14 +242,14 @@ 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, const std::string& str); +bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str); // Be conservative when using functions that // unconditionally log to debug.log! It should not be the case that an inbound // peer can fill up a user's disk with debug.log entries. template -static inline void LogPrintf_(const std::string& logging_function, const std::string& source_file, const int source_line, const BCLog::LogFlags flag, const BCLog::Level level, const char* fmt, const Args&... args) +static inline void LogPrintf_(std::string_view logging_function, std::string_view source_file, const int source_line, const BCLog::LogFlags flag, const BCLog::Level level, const char* fmt, const Args&... args) { if (LogInstance().Enabled()) { std::string log_msg; diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index a371753adf0..4113b22be87 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -58,7 +58,7 @@ static const std::string STRING_WITH_EMBEDDED_NULL_CHAR{"1"s "\0" "1"s}; /* defined in logging.cpp */ namespace BCLog { - std::string LogEscapeMessage(const std::string& str); + std::string LogEscapeMessage(std::string_view str); } BOOST_FIXTURE_TEST_SUITE(util_tests, BasicTestingSetup)