From fa2b7d8d6b3f8d53199921e1e542072441b26fab Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Thu, 19 Sep 2024 16:46:09 +0200 Subject: [PATCH] Remove redundant unterminated-logprintf tidy check --- contrib/devtools/bitcoin-tidy/CMakeLists.txt | 4 +- .../devtools/bitcoin-tidy/bitcoin-tidy.cpp | 2 - .../bitcoin-tidy/example_logprintf.cpp | 108 ------------------ contrib/devtools/bitcoin-tidy/logprintf.cpp | 60 ---------- contrib/devtools/bitcoin-tidy/logprintf.h | 29 ----- test/lint/lint-format-strings.py | 2 +- 6 files changed, 3 insertions(+), 202 deletions(-) delete mode 100644 contrib/devtools/bitcoin-tidy/example_logprintf.cpp delete mode 100644 contrib/devtools/bitcoin-tidy/logprintf.cpp delete mode 100644 contrib/devtools/bitcoin-tidy/logprintf.h diff --git a/contrib/devtools/bitcoin-tidy/CMakeLists.txt b/contrib/devtools/bitcoin-tidy/CMakeLists.txt index 95345b47829..c6f683f7aba 100644 --- a/contrib/devtools/bitcoin-tidy/CMakeLists.txt +++ b/contrib/devtools/bitcoin-tidy/CMakeLists.txt @@ -25,7 +25,7 @@ find_program(CLANG_TIDY_EXE NAMES "clang-tidy-${LLVM_VERSION_MAJOR}" "clang-tidy message(STATUS "Found LLVM ${LLVM_PACKAGE_VERSION}") message(STATUS "Found clang-tidy: ${CLANG_TIDY_EXE}") -add_library(bitcoin-tidy MODULE bitcoin-tidy.cpp logprintf.cpp nontrivial-threadlocal.cpp) +add_library(bitcoin-tidy MODULE bitcoin-tidy.cpp nontrivial-threadlocal.cpp) target_include_directories(bitcoin-tidy SYSTEM PRIVATE ${LLVM_INCLUDE_DIRS}) # Disable RTTI and exceptions as necessary @@ -58,7 +58,7 @@ else() endif() # Create a dummy library that runs clang-tidy tests as a side-effect of building -add_library(bitcoin-tidy-tests OBJECT EXCLUDE_FROM_ALL example_logprintf.cpp example_nontrivial-threadlocal.cpp) +add_library(bitcoin-tidy-tests OBJECT EXCLUDE_FROM_ALL example_nontrivial-threadlocal.cpp) add_dependencies(bitcoin-tidy-tests bitcoin-tidy) set_target_properties(bitcoin-tidy-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}") diff --git a/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp index 1ef44949737..f2658b5a586 100644 --- a/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp +++ b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp @@ -2,7 +2,6 @@ // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. -#include "logprintf.h" #include "nontrivial-threadlocal.h" #include @@ -13,7 +12,6 @@ class BitcoinModule final : public clang::tidy::ClangTidyModule public: void addCheckFactories(clang::tidy::ClangTidyCheckFactories& CheckFactories) override { - CheckFactories.registerCheck("bitcoin-unterminated-logprintf"); CheckFactories.registerCheck("bitcoin-nontrivial-threadlocal"); } }; diff --git a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp deleted file mode 100644 index dc77f668e3e..00000000000 --- a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp +++ /dev/null @@ -1,108 +0,0 @@ -// Copyright (c) 2023 Bitcoin Developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include - -// Test for bitcoin-unterminated-logprintf - -enum LogFlags { - NONE -}; - -enum Level { - None -}; - -template -static inline void LogPrintf_(const std::string& logging_function, const std::string& source_file, const int source_line, const LogFlags flag, const Level level, const char* fmt, const Args&... args) -{ -} - -#define LogPrintLevel_(category, level, ...) LogPrintf_(__func__, __FILE__, __LINE__, category, level, __VA_ARGS__) -#define LogPrintf(...) LogPrintLevel_(LogFlags::NONE, Level::None, __VA_ARGS__) - -#define LogDebug(category, ...) \ - do { \ - LogPrintf(__VA_ARGS__); \ - } while (0) - - -class CWallet -{ - std::string GetDisplayName() const - { - return "default wallet"; - } - -public: - template - void WalletLogPrintf(const char* fmt, Params... parameters) const - { - 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...); - }; -}; - -void good_func() -{ - LogPrintf("hello world!\n"); -} -void good_func2() -{ - CWallet wallet; - wallet.WalletLogPrintf("hi\n"); - ScriptPubKeyMan spkm; - spkm.WalletLogPrintf("hi\n"); - - const CWallet& walletref = wallet; - walletref.WalletLogPrintf("hi\n"); - - auto* walletptr = new CWallet(); - walletptr->WalletLogPrintf("hi\n"); - delete walletptr; -} -void bad_func() -{ - LogPrintf("hello world!"); -} -void bad_func2() -{ - LogPrintf(""); -} -void bad_func3() -{ - // Ending in "..." has no special meaning. - LogPrintf("hello world!..."); -} -void bad_func4_ignored() -{ - LogPrintf("hello world!"); // NOLINT(bitcoin-unterminated-logprintf) -} -void bad_func5() -{ - CWallet wallet; - wallet.WalletLogPrintf("hi"); - ScriptPubKeyMan spkm; - spkm.WalletLogPrintf("hi"); - - const CWallet& walletref = wallet; - walletref.WalletLogPrintf("hi"); - - auto* walletptr = new CWallet(); - walletptr->WalletLogPrintf("hi"); - delete walletptr; -} diff --git a/contrib/devtools/bitcoin-tidy/logprintf.cpp b/contrib/devtools/bitcoin-tidy/logprintf.cpp deleted file mode 100644 index 36beac28c86..00000000000 --- a/contrib/devtools/bitcoin-tidy/logprintf.cpp +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright (c) 2023 Bitcoin Developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#include "logprintf.h" - -#include -#include - - -namespace { -AST_MATCHER(clang::StringLiteral, unterminated) -{ - size_t len = Node.getLength(); - if (len > 0 && Node.getCodeUnit(len - 1) == '\n') { - return false; - } - return true; -} -} // namespace - -namespace bitcoin { - -void LogPrintfCheck::registerMatchers(clang::ast_matchers::MatchFinder* finder) -{ - using namespace clang::ast_matchers; - - /* - Logprintf(..., ..., ..., ..., ..., "foo", ...) - */ - - finder->addMatcher( - callExpr( - callee(functionDecl(hasName("LogPrintf_"))), - hasArgument(5, stringLiteral(unterminated()).bind("logstring"))), - this); - - /* - auto walletptr = &wallet; - wallet.WalletLogPrintf("foo"); - wallet->WalletLogPrintf("foo"); - */ - finder->addMatcher( - cxxMemberCallExpr( - callee(cxxMethodDecl(hasName("WalletLogPrintf"))), - hasArgument(0, stringLiteral(unterminated()).bind("logstring"))), - this); -} - -void LogPrintfCheck::check(const clang::ast_matchers::MatchFinder::MatchResult& Result) -{ - if (const clang::StringLiteral* lit = Result.Nodes.getNodeAs("logstring")) { - const clang::ASTContext& ctx = *Result.Context; - const auto user_diag = diag(lit->getEndLoc(), "Unterminated format string used with LogPrintf"); - const auto& loc = lit->getLocationOfByte(lit->getByteLength(), *Result.SourceManager, ctx.getLangOpts(), ctx.getTargetInfo()); - user_diag << clang::FixItHint::CreateInsertion(loc, "\\n"); - } -} - -} // namespace bitcoin diff --git a/contrib/devtools/bitcoin-tidy/logprintf.h b/contrib/devtools/bitcoin-tidy/logprintf.h deleted file mode 100644 index db95dfe143e..00000000000 --- a/contrib/devtools/bitcoin-tidy/logprintf.h +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) 2023 Bitcoin Developers -// Distributed under the MIT software license, see the accompanying -// file COPYING or http://www.opensource.org/licenses/mit-license.php. - -#ifndef LOGPRINTF_CHECK_H -#define LOGPRINTF_CHECK_H - -#include - -namespace bitcoin { - -// Warn about any use of LogPrintf that does not end with a newline. -class LogPrintfCheck final : public clang::tidy::ClangTidyCheck -{ -public: - LogPrintfCheck(clang::StringRef Name, clang::tidy::ClangTidyContext* Context) - : clang::tidy::ClangTidyCheck(Name, Context) {} - - bool isLanguageVersionSupported(const clang::LangOptions& LangOpts) const override - { - return LangOpts.CPlusPlus; - } - void registerMatchers(clang::ast_matchers::MatchFinder* Finder) override; - void check(const clang::ast_matchers::MatchFinder::MatchResult& Result) override; -}; - -} // namespace bitcoin - -#endif // LOGPRINTF_CHECK_H diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index a809851ec63..86a17fb0f8c 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -62,7 +62,7 @@ def main(): matching_files_filtered = [] for matching_file in matching_files: - if not re.search('^src/(leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp)|contrib/devtools/bitcoin-tidy/example_logprintf.cpp', matching_file): + if not re.search('^src/(leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp)', matching_file): matching_files_filtered.append(matching_file) matching_files_filtered.sort()