log: Use ConstevalFormatString

MarcoFalke · MarcoFalke · commit facbcd4cef88 · 2024-09-17T18:21:23.000+02:00
This changes all logging (including the wallet logging) to produce a
ConstevalFormatString at compile time, so that the format string can be
validated at compile-time.

Also, while touching the wallet logging, avoid a copy of the template
Params by using const Params&amp;.
diff --git a/src/logging.h b/src/logging.h
@@ -246,21 +246,21 @@ static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level leve
 bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);
 
 template <typename... 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)
+inline void LogPrintFormatInternal(std::string_view logging_function, std::string_view source_file, const int source_line, const BCLog::LogFlags flag, const BCLog::Level level, 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) {
             /* Original format string will have newline so don't add one here */
-            log_msg = "Error \"" + std::string(fmterr.what()) + "\" while formatting log message: " + fmt;
+            log_msg = "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: " + fmt.fmt;
         }
         LogInstance().LogPrintStr(log_msg, logging_function, source_file, source_line, flag, level);
     }
 }
 
-#define LogPrintLevel_(category, level, ...) LogPrintf_(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__)
+#define LogPrintLevel_(category, level, ...) LogPrintFormatInternal(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__)
 
 // Log unconditionally.
 // Be conservative when using functions that unconditionally log to debug.log!
diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h
@@ -254,9 +254,9 @@ class ScriptPubKeyMan
 
     /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */
     template <typename... Params>
-    void WalletLogPrintf(const char* fmt, Params... parameters) const
+    void WalletLogPrintf(util::ConstevalFormatString<sizeof...(Params)> wallet_fmt, const Params&... params) const
     {
-        LogPrintf(("%s " + std::string{fmt}).c_str(), m_storage.GetDisplayName(), parameters...);
+        LogInfo("%s %s", m_storage.GetDisplayName(), tfm::format(wallet_fmt, params...));
     };
 
     /** Watch-only address added */
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
@@ -927,9 +927,9 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
 
     /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */
     template <typename... Params>
-    void WalletLogPrintf(const char* fmt, Params... parameters) const
+    void WalletLogPrintf(util::ConstevalFormatString<sizeof...(Params)> wallet_fmt, const Params&... params) const
     {
-        LogPrintf(("%s " + std::string{fmt}).c_str(), GetDisplayName(), parameters...);
+        LogInfo("%s %s", GetDisplayName(), tfm::format(wallet_fmt, params...));
     };
 
     /** Upgrade the wallet */
diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py
@@ -17,15 +17,7 @@
 
 FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS = [
     'tfm::format,1',  # Assuming tfm::::format(std::ostream&, ...
-    'LogError,0',
-    'LogWarning,0',
-    'LogInfo,0',
-    'LogDebug,1',
-    'LogTrace,1',
-    'LogPrintf,0',
-    'LogPrintLevel,2',
     'strprintf,0',
-    'WalletLogPrintf,0',
 ]
 RUN_LINT_FILE = 'test/lint/run-lint-format-strings.py'
 
diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py
@@ -15,11 +15,6 @@
 FALSE_POSITIVES = [
     ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"),
     ("src/test/translation_tests.cpp", "strprintf(format, arg)"),
-    ("src/validationinterface.cpp", "LogDebug(BCLog::VALIDATION, fmt \"\\n\", __VA_ARGS__)"),
-    ("src/wallet/wallet.h", "WalletLogPrintf(const char* fmt, Params... parameters)"),
-    ("src/wallet/wallet.h", "LogPrintf((\"%s \" + std::string{fmt}).c_str(), GetDisplayName(), parameters...)"),
-    ("src/wallet/scriptpubkeyman.h", "WalletLogPrintf(const char* fmt, Params... parameters)"),
-    ("src/wallet/scriptpubkeyman.h", "LogPrintf((\"%s \" + std::string{fmt}).c_str(), m_storage.GetDisplayName(), parameters...)"),
 ]
 
 

Original file line number	Diff line number	Diff line change
`@@ -246,21 +246,21 @@ static inline bool LogAcceptCategory(BCLog::LogFlags category, BCLog::Level leve`
`246`	`246`	`bool GetLogCategory(BCLog::LogFlags& flag, std::string_view str);`
`247`	`247`
`248`	`248`	`template <typename... Args>`
`249`		`-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)`
	`249`	`+inline void LogPrintFormatInternal(std::string_view logging_function, std::string_view source_file, const int source_line, const BCLog::LogFlags flag, const BCLog::Level level, util::ConstevalFormatString<sizeof...(Args)> fmt, const Args&... args)`
`250`	`250`	`{`
`251`	`251`	`if (LogInstance().Enabled()) {`
`252`	`252`	`std::string log_msg;`
`253`	`253`	`try {`
`254`	`254`	`log_msg = tfm::format(fmt, args...);`
`255`	`255`	`} catch (tinyformat::format_error& fmterr) {`
`256`	`256`	`/* Original format string will have newline so don't add one here */`
`257`		`- log_msg = "Error \"" + std::string(fmterr.what()) + "\" while formatting log message: " + fmt;`
	`257`	`+ log_msg = "Error \"" + std::string{fmterr.what()} + "\" while formatting log message: " + fmt.fmt;`
`258`	`258`	`}`
`259`	`259`	`LogInstance().LogPrintStr(log_msg, logging_function, source_file, source_line, flag, level);`
`260`	`260`	`}`
`261`	`261`	`}`
`262`	`262`
`263`		`-#define LogPrintLevel_(category, level, ...) LogPrintf_(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__)`
	`263`	`+#define LogPrintLevel_(category, level, ...) LogPrintFormatInternal(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__)`
`264`	`264`
`265`	`265`	`// Log unconditionally.`
`266`	`266`	`// Be conservative when using functions that unconditionally log to debug.log!`