diff options
-rwxr-xr-x | ci/test/00_setup_env.sh | 1 | ||||
-rwxr-xr-x | ci/test/01_base_install.sh | 6 | ||||
-rwxr-xr-x | ci/test/06_script_b.sh | 10 | ||||
-rw-r--r-- | contrib/devtools/bitcoin-tidy/CMakeLists.txt | 45 | ||||
-rw-r--r-- | contrib/devtools/bitcoin-tidy/README | 8 | ||||
-rw-r--r-- | contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp | 22 | ||||
-rw-r--r-- | contrib/devtools/bitcoin-tidy/example_logprintf.cpp | 91 | ||||
-rw-r--r-- | contrib/devtools/bitcoin-tidy/logprintf.cpp | 62 | ||||
-rw-r--r-- | contrib/devtools/bitcoin-tidy/logprintf.h | 28 | ||||
-rw-r--r-- | src/.clang-tidy | 1 | ||||
-rw-r--r-- | src/dbwrapper.cpp | 2 | ||||
-rw-r--r-- | src/index/base.cpp | 4 | ||||
-rw-r--r-- | src/init.cpp | 2 | ||||
-rw-r--r-- | src/net_processing.cpp | 2 | ||||
-rw-r--r-- | src/node/utxo_snapshot.cpp | 4 | ||||
-rw-r--r-- | src/test/logging_tests.cpp | 8 | ||||
-rw-r--r-- | src/test/util/chainstate.h | 2 | ||||
-rw-r--r-- | src/validation.cpp | 18 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 2 | ||||
-rwxr-xr-x | test/lint/lint-format-strings.py | 2 | ||||
-rwxr-xr-x | test/lint/lint-include-guards.py | 3 | ||||
-rwxr-xr-x | test/lint/lint-includes.py | 3 | ||||
-rwxr-xr-x | test/lint/lint-logs.py | 34 |
23 files changed, 294 insertions, 66 deletions
diff --git a/ci/test/00_setup_env.sh b/ci/test/00_setup_env.sh index 722a877890..ba8fc861e8 100755 --- a/ci/test/00_setup_env.sh +++ b/ci/test/00_setup_env.sh @@ -67,7 +67,6 @@ export BASE_BUILD_DIR=${BASE_BUILD_DIR:-$BASE_SCRATCH_DIR/build} # The folder for previous release binaries. # This folder exists only on the ci guest, and on the ci host as a volume. export PREVIOUS_RELEASES_DIR=${PREVIOUS_RELEASES_DIR:-$BASE_ROOT_DIR/releases/$HOST} -export DIR_IWYU="${BASE_SCRATCH_DIR}/iwyu" export SDK_URL=${SDK_URL:-https://bitcoincore.org/depends-sources/sdks} export CI_BASE_PACKAGES=${CI_BASE_PACKAGES:-build-essential libtool autotools-dev automake pkg-config bsdmainutils curl ca-certificates ccache python3 rsync git procps bison} export GOAL=${GOAL:-install} diff --git a/ci/test/01_base_install.sh b/ci/test/01_base_install.sh index 76cde42161..83213b2856 100755 --- a/ci/test/01_base_install.sh +++ b/ci/test/01_base_install.sh @@ -72,9 +72,9 @@ if [[ ${USE_MEMORY_SANITIZER} == "true" ]]; then fi if [[ "${RUN_TIDY}" == "true" ]]; then - git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_16 "${DIR_IWYU}"/include-what-you-use - cmake -B "${DIR_IWYU}"/build/ -G 'Unix Makefiles' -DCMAKE_PREFIX_PATH=/usr/lib/llvm-16 -S "${DIR_IWYU}"/include-what-you-use - make -C "${DIR_IWYU}"/build/ install "$MAKEJOBS" + git clone --depth=1 https://github.com/include-what-you-use/include-what-you-use -b clang_16 /include-what-you-use + cmake -B /iwyu-build/ -G 'Unix Makefiles' -DCMAKE_PREFIX_PATH=/usr/lib/llvm-16 -S /include-what-you-use + make -C /iwyu-build/ install "$MAKEJOBS" fi mkdir -p "${DEPENDS_DIR}/SDKs" "${DEPENDS_DIR}/sdk-sources" diff --git a/ci/test/06_script_b.sh b/ci/test/06_script_b.sh index 6e921d3377..02358db789 100755 --- a/ci/test/06_script_b.sh +++ b/ci/test/06_script_b.sh @@ -148,9 +148,13 @@ if [ "$RUN_FUNCTIONAL_TESTS" = "true" ]; then fi if [ "${RUN_TIDY}" = "true" ]; then + cmake -B /tidy-build -DLLVM_DIR=/usr/lib/llvm-16/cmake -DCMAKE_BUILD_TYPE=Release -S "${BASE_ROOT_DIR}"/contrib/devtools/bitcoin-tidy + cmake --build /tidy-build "$MAKEJOBS" + cmake --build /tidy-build --target bitcoin-tidy-tests "$MAKEJOBS" + set -eo pipefail cd "${BASE_BUILD_DIR}/bitcoin-$HOST/src/" - ( run-clang-tidy-16 -quiet "${MAKEJOBS}" ) | grep -C5 "error" + ( run-clang-tidy-16 -quiet -load="/tidy-build/libbitcoin-tidy.so" "${MAKEJOBS}" ) | grep -C5 "error" # Filter out files by regex here, because regex may not be # accepted in src/.bear-tidy-config # Filter out: @@ -158,13 +162,13 @@ if [ "${RUN_TIDY}" = "true" ]; then jq 'map(select(.file | test("src/qt/qrc_.*\\.cpp$|/moc_.*\\.cpp$") | not))' ../compile_commands.json > tmp.json mv tmp.json ../compile_commands.json cd "${BASE_BUILD_DIR}/bitcoin-$HOST/" - python3 "${DIR_IWYU}/include-what-you-use/iwyu_tool.py" \ + python3 "/include-what-you-use/iwyu_tool.py" \ -p . "${MAKEJOBS}" \ -- -Xiwyu --cxx17ns -Xiwyu --mapping_file="${BASE_BUILD_DIR}/bitcoin-$HOST/contrib/devtools/iwyu/bitcoin.core.imp" \ -Xiwyu --max_line_length=160 \ 2>&1 | tee /tmp/iwyu_ci.out cd "${BASE_ROOT_DIR}/src" - python3 "${DIR_IWYU}/include-what-you-use/fix_includes.py" --nosafe_headers < /tmp/iwyu_ci.out + python3 "/include-what-you-use/fix_includes.py" --nosafe_headers < /tmp/iwyu_ci.out git --no-pager diff fi diff --git a/contrib/devtools/bitcoin-tidy/CMakeLists.txt b/contrib/devtools/bitcoin-tidy/CMakeLists.txt new file mode 100644 index 0000000000..9ed18696d4 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/CMakeLists.txt @@ -0,0 +1,45 @@ +cmake_minimum_required(VERSION 3.9) + +project(bitcoin-tidy VERSION 1.0.0 DESCRIPTION "clang-tidy checks for Bitcoin Core") + +include(GNUInstallDirs) + +set(CMAKE_CXX_STANDARD 17) +set(CMAKE_CXX_STANDARD_REQUIRED True) +set(CMAKE_CXX_EXTENSIONS False) + +# TODO: Figure out how to avoid the terminfo check +find_package(LLVM REQUIRED CONFIG) +find_program(CLANG_TIDY_EXE NAMES "clang-tidy-${LLVM_VERSION_MAJOR}" "clang-tidy" HINTS ${LLVM_TOOLS_BINARY_DIR}) +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) +target_include_directories(bitcoin-tidy SYSTEM PRIVATE ${LLVM_INCLUDE_DIRS}) + +# Disable RTTI and exceptions as necessary +if (MSVC) + target_compile_options(bitcoin-tidy PRIVATE /GR-) +else() + target_compile_options(bitcoin-tidy PRIVATE -fno-rtti) + target_compile_options(bitcoin-tidy PRIVATE -fno-exceptions) +endif() + +# Add warnings +if (MSVC) + target_compile_options(bitcoin-tidy PRIVATE /W4) +else() + target_compile_options(bitcoin-tidy PRIVATE -Wall) + target_compile_options(bitcoin-tidy PRIVATE -Wextra) +endif() + +set(CLANG_TIDY_COMMAND "${CLANG_TIDY_EXE}" "--load=${CMAKE_BINARY_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}bitcoin-tidy${CMAKE_SHARED_LIBRARY_SUFFIX}" "-checks=-*,bitcoin-*") + +# 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) +add_dependencies(bitcoin-tidy-tests bitcoin-tidy) + +set_target_properties(bitcoin-tidy-tests PROPERTIES CXX_CLANG_TIDY "${CLANG_TIDY_COMMAND}") + + +install(TARGETS bitcoin-tidy LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}) diff --git a/contrib/devtools/bitcoin-tidy/README b/contrib/devtools/bitcoin-tidy/README new file mode 100644 index 0000000000..1669fe98f5 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/README @@ -0,0 +1,8 @@ +# Bitcoin Tidy + +Example Usage: + +```bash +cmake -S . -B build -DLLVM_DIR=/path/to/lib/cmake/llvm -DCMAKE_BUILD_TYPE=Release +make -C build -j$(nproc) +``` diff --git a/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp new file mode 100644 index 0000000000..0f34d37793 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/bitcoin-tidy.cpp @@ -0,0 +1,22 @@ +// 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 <clang-tidy/ClangTidyModule.h> +#include <clang-tidy/ClangTidyModuleRegistry.h> + +class BitcoinModule final : public clang::tidy::ClangTidyModule +{ +public: + void addCheckFactories(clang::tidy::ClangTidyCheckFactories& CheckFactories) override + { + CheckFactories.registerCheck<bitcoin::LogPrintfCheck>("bitcoin-unterminated-logprintf"); + } +}; + +static clang::tidy::ClangTidyModuleRegistry::Add<BitcoinModule> + X("bitcoin-module", "Adds bitcoin checks."); + +volatile int BitcoinModuleAnchorSource = 0; diff --git a/contrib/devtools/bitcoin-tidy/example_logprintf.cpp b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp new file mode 100644 index 0000000000..a3d2768964 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/example_logprintf.cpp @@ -0,0 +1,91 @@ +// 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. + +// Warn about any use of LogPrintf that does not end with a newline. +#include <string> + +enum LogFlags { + NONE +}; + +enum Level { + None +}; + +template <typename... Args> +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__) + +// 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__); \ + } while (0) + + +class CWallet +{ + std::string GetDisplayName() const + { + return "default wallet"; + } + +public: + template <typename... Params> + void WalletLogPrintf(std::string fmt, Params... parameters) const + { + LogPrintf(("%s " + fmt).c_str(), GetDisplayName(), parameters...); + }; +}; + +void good_func() +{ + LogPrintf("hello world!\n"); +} +void good_func2() +{ + CWallet wallet; + wallet.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"); + + 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 new file mode 100644 index 0000000000..1690c8fde0 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/logprintf.cpp @@ -0,0 +1,62 @@ +// 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 <clang/AST/ASTContext.h> +#include <clang/ASTMatchers/ASTMatchFinder.h> + + +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); + + /* + 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); +} + +void LogPrintfCheck::check(const clang::ast_matchers::MatchFinder::MatchResult& Result) +{ + if (const clang::StringLiteral* lit = Result.Nodes.getNodeAs<clang::StringLiteral>("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 new file mode 100644 index 0000000000..466849ef01 --- /dev/null +++ b/contrib/devtools/bitcoin-tidy/logprintf.h @@ -0,0 +1,28 @@ +// 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 <clang-tidy/ClangTidyCheck.h> + +namespace bitcoin { + +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/src/.clang-tidy b/src/.clang-tidy index 84c9d5fb3a..b4d50135dd 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -1,5 +1,6 @@ Checks: ' -*, +bitcoin-unterminated-logprintf, bugprone-argument-comment, bugprone-use-after-move, misc-unused-using-decls, diff --git a/src/dbwrapper.cpp b/src/dbwrapper.cpp index 2aade14ef4..7cf6bef90f 100644 --- a/src/dbwrapper.cpp +++ b/src/dbwrapper.cpp @@ -76,7 +76,7 @@ public: assert(p <= limit); base[std::min(bufsize - 1, (int)(p - base))] = '\0'; - LogPrintLevel(BCLog::LEVELDB, BCLog::Level::Debug, "%s", base); /* Continued */ + LogPrintLevel(BCLog::LEVELDB, BCLog::Level::Debug, "%s", base); // NOLINT(bitcoin-unterminated-logprintf) if (base != buffer) { delete[] base; } diff --git a/src/index/base.cpp b/src/index/base.cpp index 55fb154d99..f18205a76f 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -270,7 +270,7 @@ void BaseIndex::BlockConnected(const std::shared_ptr<const CBlock>& block, const // in the ValidationInterface queue backlog even after the sync thread has caught up to the // new chain tip. In this unlikely event, log a warning and let the queue clear. if (best_block_index->GetAncestor(pindex->nHeight - 1) != pindex->pprev) { - LogPrintf("%s: WARNING: Block %s does not connect to an ancestor of " /* Continued */ + LogPrintf("%s: WARNING: Block %s does not connect to an ancestor of " "known best chain (tip=%s); not updating index\n", __func__, pindex->GetBlockHash().ToString(), best_block_index->GetBlockHash().ToString()); @@ -322,7 +322,7 @@ void BaseIndex::ChainStateFlushed(const CBlockLocator& locator) // event, log a warning and let the queue clear. const CBlockIndex* best_block_index = m_best_block_index.load(); if (best_block_index->GetAncestor(locator_tip_index->nHeight) != locator_tip_index) { - LogPrintf("%s: WARNING: Locator contains block (hash=%s) not on known best " /* Continued */ + LogPrintf("%s: WARNING: Locator contains block (hash=%s) not on known best " "chain (tip=%s); not writing index locator\n", __func__, locator_tip_hash.ToString(), best_block_index->GetBlockHash().ToString()); diff --git a/src/init.cpp b/src/init.cpp index 4d526bd0de..65f2d221bd 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1082,7 +1082,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // Warn about relative -datadir path. if (args.IsArgSet("-datadir") && !args.GetPathArg("-datadir").is_absolute()) { - LogPrintf("Warning: relative datadir option '%s' specified, which will be interpreted relative to the " /* Continued */ + LogPrintf("Warning: relative datadir option '%s' specified, which will be interpreted relative to the " "current working directory '%s'. This is fragile, because if bitcoin is started in the future " "from a different location, it will be unable to locate the current data files. There could " "also be data loss if bitcoin is started while in a temporary directory.\n", diff --git a/src/net_processing.cpp b/src/net_processing.cpp index e497ba47e5..5bb9647f35 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -3048,7 +3048,7 @@ bool PeerManagerImpl::PrepareBlockFilterRequest(CNode& node, Peer& peer, uint32_t stop_height = stop_index->nHeight; if (start_height > stop_height) { - LogPrint(BCLog::NET, "peer %d sent invalid getcfilters/getcfheaders with " /* Continued */ + LogPrint(BCLog::NET, "peer %d sent invalid getcfilters/getcfheaders with " "start height %d and stop height %d\n", node.GetId(), start_height, stop_height); node.fDisconnect = true; diff --git a/src/node/utxo_snapshot.cpp b/src/node/utxo_snapshot.cpp index 036a25d0a5..976421e455 100644 --- a/src/node/utxo_snapshot.cpp +++ b/src/node/utxo_snapshot.cpp @@ -49,7 +49,7 @@ bool WriteSnapshotBaseBlockhash(Chainstate& snapshot_chainstate) std::optional<uint256> ReadSnapshotBaseBlockhash(fs::path chaindir) { if (!fs::exists(chaindir)) { - LogPrintf("[snapshot] cannot read base blockhash: no chainstate dir " /* Continued */ + LogPrintf("[snapshot] cannot read base blockhash: no chainstate dir " "exists at path %s\n", fs::PathToString(chaindir)); return std::nullopt; } @@ -57,7 +57,7 @@ std::optional<uint256> ReadSnapshotBaseBlockhash(fs::path chaindir) const std::string read_from_str = fs::PathToString(read_from); if (!fs::exists(read_from)) { - LogPrintf("[snapshot] snapshot chainstate dir is malformed! no base blockhash file " /* Continued */ + LogPrintf("[snapshot] snapshot chainstate dir is malformed! no base blockhash file " "exists at path %s. Try deleting %s and calling loadtxoutset again?\n", fs::PathToString(chaindir), read_from_str); return std::nullopt; diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index 2699d316da..e448805e69 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -83,10 +83,10 @@ BOOST_AUTO_TEST_CASE(logging_timer) BOOST_FIXTURE_TEST_CASE(logging_LogPrintf_, LogSetup) { LogInstance().m_log_sourcelocations = true; - LogPrintf_("fn1", "src1", 1, BCLog::LogFlags::NET, BCLog::Level::Debug, "foo1: %s", "bar1\n"); - LogPrintf_("fn2", "src2", 2, BCLog::LogFlags::NET, BCLog::Level::None, "foo2: %s", "bar2\n"); - LogPrintf_("fn3", "src3", 3, BCLog::LogFlags::NONE, BCLog::Level::Debug, "foo3: %s", "bar3\n"); - LogPrintf_("fn4", "src4", 4, BCLog::LogFlags::NONE, BCLog::Level::None, "foo4: %s", "bar4\n"); + LogPrintf_("fn1", "src1", 1, BCLog::LogFlags::NET, BCLog::Level::Debug, "foo1: %s\n", "bar1"); + LogPrintf_("fn2", "src2", 2, BCLog::LogFlags::NET, BCLog::Level::None, "foo2: %s\n", "bar2"); + LogPrintf_("fn3", "src3", 3, BCLog::LogFlags::NONE, BCLog::Level::Debug, "foo3: %s\n", "bar3"); + LogPrintf_("fn4", "src4", 4, BCLog::LogFlags::NONE, BCLog::Level::None, "foo4: %s\n", "bar4"); std::ifstream file{tmp_log_path}; std::vector<std::string> log_lines; for (std::string log; std::getline(file, log);) { diff --git a/src/test/util/chainstate.h b/src/test/util/chainstate.h index 9ff2c08807..7f55916870 100644 --- a/src/test/util/chainstate.h +++ b/src/test/util/chainstate.h @@ -50,7 +50,7 @@ CreateAndActivateUTXOSnapshot( UniValue result = CreateUTXOSnapshot( node, node.chainman->ActiveChainstate(), auto_outfile, snapshot_path, snapshot_path); LogPrintf( - "Wrote UTXO snapshot to %s: %s", fs::PathToString(snapshot_path.make_preferred()), result.write()); + "Wrote UTXO snapshot to %s: %s\n", fs::PathToString(snapshot_path.make_preferred()), result.write()); // Read the written snapshot in and then activate it. // diff --git a/src/validation.cpp b/src/validation.cpp index cd6654abe4..396133a4cf 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3102,7 +3102,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< // Belt-and-suspenders check that we aren't attempting to advance the background // chainstate past the snapshot base block. if (WITH_LOCK(::cs_main, return m_disabled)) { - LogPrintf("m_disabled is set - this chainstate should not be in operation. " /* Continued */ + LogPrintf("m_disabled is set - this chainstate should not be in operation. " "Please report this as a bug. %s\n", PACKAGE_BUGREPORT); return false; } @@ -5226,7 +5226,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( auto maybe_au_data = ExpectedAssumeutxo(base_height, GetParams()); if (!maybe_au_data) { - LogPrintf("[snapshot] assumeutxo height in snapshot metadata not recognized " /* Continued */ + LogPrintf("[snapshot] assumeutxo height in snapshot metadata not recognized " "(%d) - refusing to load snapshot\n", base_height); return false; } @@ -5473,8 +5473,8 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() }; if (index_new.GetBlockHash() != snapshot_blockhash) { - LogPrintf("[snapshot] supposed base block %s does not match the " /* Continued */ - "snapshot base block %s (height %d). Snapshot is not valid.", + LogPrintf("[snapshot] supposed base block %s does not match the " + "snapshot base block %s (height %d). Snapshot is not valid.\n", index_new.ToString(), snapshot_blockhash.ToString(), snapshot_base_height); handle_invalid_snapshot(); return SnapshotCompletionResult::BASE_BLOCKHASH_MISMATCH; @@ -5494,7 +5494,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() auto maybe_au_data = ExpectedAssumeutxo(curr_height, m_options.chainparams); if (!maybe_au_data) { - LogPrintf("[snapshot] assumeutxo data not found for height " /* Continued */ + LogPrintf("[snapshot] assumeutxo data not found for height " "(%d) - refusing to validate snapshot\n", curr_height); handle_invalid_snapshot(); return SnapshotCompletionResult::MISSING_CHAINPARAMS; @@ -5502,7 +5502,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() const AssumeutxoData& au_data = *maybe_au_data; std::optional<CCoinsStats> maybe_ibd_stats; - LogPrintf("[snapshot] computing UTXO stats for background chainstate to validate " /* Continued */ + LogPrintf("[snapshot] computing UTXO stats for background chainstate to validate " "snapshot - this could take a few minutes\n"); try { maybe_ibd_stats = ComputeUTXOStats( @@ -5740,7 +5740,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() // is in-memory, in which case we can't do on-disk cleanup. You'd better be // in a unittest! if (!ibd_chainstate_path_maybe || !snapshot_chainstate_path_maybe) { - LogPrintf("[snapshot] snapshot chainstate cleanup cannot happen with " /* Continued */ + LogPrintf("[snapshot] snapshot chainstate cleanup cannot happen with " "in-memory chainstates. You are testing, right?\n"); return false; } @@ -5783,7 +5783,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() throw; } - LogPrintf("[snapshot] moving snapshot chainstate (%s) to " /* Continued */ + LogPrintf("[snapshot] moving snapshot chainstate (%s) to " "default chainstate directory (%s)\n", fs::PathToString(snapshot_chainstate_path), fs::PathToString(ibd_chainstate_path)); @@ -5797,7 +5797,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() if (!DeleteCoinsDBFromDisk(tmp_old, /*is_snapshot=*/false)) { // No need to FatalError because once the unneeded bg chainstate data is // moved, it will not interfere with subsequent initialization. - LogPrintf("Deletion of %s failed. Please remove it manually, as the " /* Continued */ + LogPrintf("Deletion of %s failed. Please remove it manually, as the " "directory is now unnecessary.\n", fs::PathToString(tmp_old)); } else { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 8fa93b97d6..6b2755ea53 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2319,7 +2319,7 @@ OutputType CWallet::TransactionChangeType(const std::optional<OutputType>& chang void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector<std::pair<std::string, std::string>> orderForm) { LOCK(cs_wallet); - WalletLogPrintf("CommitTransaction:\n%s", tx->ToString()); /* Continued */ + WalletLogPrintf("CommitTransaction:\n%s", tx->ToString()); // Add tx to wallet, because if it has change it's also ours, // otherwise just for transaction history. diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index 43addab2f3..5ac5840ecf 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -77,7 +77,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)', matching_file): + if not re.search('^src/(leveldb|secp256k1|minisketch|tinyformat|test/fuzz/strprintf.cpp)|contrib/devtools/bitcoin-tidy/example_logprintf.cpp', matching_file): matching_files_filtered.append(matching_file) matching_files_filtered.sort() diff --git a/test/lint/lint-include-guards.py b/test/lint/lint-include-guards.py index 5867aae028..48b918e9da 100755 --- a/test/lint/lint-include-guards.py +++ b/test/lint/lint-include-guards.py @@ -17,7 +17,8 @@ from typing import List HEADER_ID_PREFIX = 'BITCOIN_' HEADER_ID_SUFFIX = '_H' -EXCLUDE_FILES_WITH_PREFIX = ['src/crypto/ctaes', +EXCLUDE_FILES_WITH_PREFIX = ['contrib/devtools/bitcoin-tidy', + 'src/crypto/ctaes', 'src/leveldb', 'src/crc32c', 'src/secp256k1', diff --git a/test/lint/lint-includes.py b/test/lint/lint-includes.py index b14caa4855..8e79ba5121 100755 --- a/test/lint/lint-includes.py +++ b/test/lint/lint-includes.py @@ -15,7 +15,8 @@ import sys from subprocess import check_output, CalledProcessError -EXCLUDED_DIRS = ["src/leveldb/", +EXCLUDED_DIRS = ["contrib/devtools/bitcoin-tidy/", + "src/leveldb/", "src/crc32c/", "src/secp256k1/", "src/minisketch/", diff --git a/test/lint/lint-logs.py b/test/lint/lint-logs.py deleted file mode 100755 index de04a1aeca..0000000000 --- a/test/lint/lint-logs.py +++ /dev/null @@ -1,34 +0,0 @@ -#!/usr/bin/env python3 -# -# Copyright (c) 2018-2022 The Bitcoin Core developers -# Distributed under the MIT software license, see the accompanying -# file COPYING or http://www.opensource.org/licenses/mit-license.php. -# -# Check that all logs are terminated with '\n' -# -# Some logs are continued over multiple lines. They should be explicitly -# commented with /* Continued */ - -import re -import sys - -from subprocess import check_output - - -def main(): - logs_list = check_output(["git", "grep", "--extended-regexp", r"(LogPrintLevel|LogPrintfCategory|LogPrintf?)\(", "--", "*.cpp"], text=True, encoding="utf8").splitlines() - - unterminated_logs = [line for line in logs_list if not re.search(r'(\\n"|/\* Continued \*/)', line)] - - if unterminated_logs != []: - print("All calls to LogPrintf(), LogPrintfCategory(), LogPrint(), LogPrintLevel(), and WalletLogPrintf() should be terminated with \"\\n\".") - print("") - - for line in unterminated_logs: - print(line) - - sys.exit(1) - - -if __name__ == "__main__": - main() |