diff --git a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp index a3d2768964c..a12a666c086 100644 --- a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp +++ b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp @@ -2,9 +2,10 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -// Warn about any use of LogPrintf that does not end with a newline. #include +// Test for bitcoin-unterminated-logprintf + enum LogFlags { NONE }; @@ -21,8 +22,6 @@ static inline void LogPrintf_(const std::string& logging_function, const std::st #define LogPrintLevel_(category, level, ...) LogPrintf_(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__) #define LogPrintf(...) LogPrintLevel_(LogFlags::NONE, Level::None, __VA_ARGS__) -// Use a macro instead of a function for conditional logging to prevent -// evaluating arguments when logging for the category is not enabled. #define LogPrint(category, ...) \ do { \ LogPrintf(__VA_ARGS__); \ @@ -38,9 +37,23 @@ class CWallet public: template - void WalletLogPrintf(std::string fmt, Params... parameters) const + void WalletLogPrintf(const char* fmt, Params... parameters) const { - LogPrintf(("%s " + fmt).c_str(), GetDisplayName(), parameters...); + LogPrintf(("%s " + std::string{fmt}).c_str(), GetDisplayName(), parameters...); + }; +}; + +struct ScriptPubKeyMan +{ + std::string GetDisplayName() const + { + return "default wallet"; + } + + template + void WalletLogPrintf(const char* fmt, Params... parameters) const + { + LogPrintf(("%s " + std::string{fmt}).c_str(), GetDisplayName(), parameters...); }; }; @@ -52,6 +65,8 @@ void good_func2() { CWallet wallet; wallet.WalletLogPrintf("hi\n"); + ScriptPubKeyMan spkm; + spkm.WalletLogPrintf("hi\n"); const CWallet& walletref = wallet; walletref.WalletLogPrintf("hi\n"); @@ -81,6 +96,8 @@ void bad_func5() { CWallet wallet; wallet.WalletLogPrintf("hi"); + ScriptPubKeyMan spkm; + spkm.WalletLogPrintf("hi"); const CWallet& walletref = wallet; walletref.WalletLogPrintf("hi"); diff --git a/contrib/devtools/bitcoin-tidy/logprintf.cpp b/contrib/devtools/bitcoin-tidy/logprintf.cpp index 1690c8fde0a..36beac28c86 100644 --- a/contrib/devtools/bitcoin-tidy/logprintf.cpp +++ b/contrib/devtools/bitcoin-tidy/logprintf.cpp @@ -36,14 +36,12 @@ void LogPrintfCheck::registerMatchers(clang::ast_matchers::MatchFinder* finder) this); /* - CWallet wallet; auto walletptr = &wallet; wallet.WalletLogPrintf("foo"); wallet->WalletLogPrintf("foo"); */ finder->addMatcher( cxxMemberCallExpr( - thisPointerType(qualType(hasDeclaration(cxxRecordDecl(hasName("CWallet"))))), callee(cxxMethodDecl(hasName("WalletLogPrintf"))), hasArgument(0, stringLiteral(unterminated()).bind("logstring"))), this); diff --git a/contrib/devtools/bitcoin-tidy/logprintf.h b/contrib/devtools/bitcoin-tidy/logprintf.h index 466849ef011..db95dfe143e 100644 --- a/contrib/devtools/bitcoin-tidy/logprintf.h +++ b/contrib/devtools/bitcoin-tidy/logprintf.h @@ -9,6 +9,7 @@ namespace bitcoin { +// Warn about any use of LogPrintf that does not end with a newline. class LogPrintfCheck final : public clang::tidy::ClangTidyCheck { public: diff --git a/src/.clang-tidy b/src/.clang-tidy index b4d50135dd0..4deb5a85a5e 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -1,6 +1,6 @@ Checks: ' -*, -bitcoin-unterminated-logprintf, +bitcoin-*, bugprone-argument-comment, bugprone-use-after-move, misc-unused-using-decls, diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index f03673e9d37..ec7b017720f 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -250,9 +250,10 @@ public: virtual std::unordered_set GetScriptPubKeys() const { return {}; }; /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */ - template - void WalletLogPrintf(std::string fmt, Params... parameters) const { - LogPrintf(("%s " + fmt).c_str(), m_storage.GetDisplayName(), parameters...); + template + void WalletLogPrintf(const char* fmt, Params... parameters) const + { + LogPrintf(("%s " + std::string{fmt}).c_str(), m_storage.GetDisplayName(), parameters...); }; /** Watch-only address added */ diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index fd9a3e5a307..52f6e8e2436 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2320,7 +2320,7 @@ OutputType CWallet::TransactionChangeType(const std::optional& chang void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm) { LOCK(cs_wallet); - WalletLogPrintf("CommitTransaction:\n%s", tx->ToString()); + WalletLogPrintf("CommitTransaction:\n%s", tx->ToString()); // NOLINT(bitcoin-unterminated-logprintf) // Add tx to wallet, because if it has change it's also ours, // otherwise just for transaction history. diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 91c8e89d2d9..e2be6a206a4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -891,9 +891,10 @@ public: }; /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */ - template - void WalletLogPrintf(std::string fmt, Params... parameters) const { - LogPrintf(("%s " + fmt).c_str(), GetDisplayName(), parameters...); + template + void WalletLogPrintf(const char* fmt, Params... parameters) const + { + LogPrintf(("%s " + std::string{fmt}).c_str(), GetDisplayName(), parameters...); }; /** Upgrade the wallet */ diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py index ed98b1b2f81..244bf5956f0 100755 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -20,10 +20,10 @@ FALSE_POSITIVES = [ ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"), ("src/test/translation_tests.cpp", "strprintf(format, arg)"), ("src/validationinterface.cpp", "LogPrint(BCLog::VALIDATION, fmt \"\\n\", __VA_ARGS__)"), - ("src/wallet/wallet.h", "WalletLogPrintf(std::string fmt, Params... parameters)"), - ("src/wallet/wallet.h", "LogPrintf((\"%s \" + fmt).c_str(), GetDisplayName(), parameters...)"), - ("src/wallet/scriptpubkeyman.h", "WalletLogPrintf(std::string fmt, Params... parameters)"), - ("src/wallet/scriptpubkeyman.h", "LogPrintf((\"%s \" + fmt).c_str(), m_storage.GetDisplayName(), parameters...)"), + ("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...)"), ]