From faf2759c8c4507adacf2618f23c1f445f490c012 Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Mon, 10 Nov 2025 12:50:15 +0100 Subject: [PATCH 1/2] test: [refactor] Use reference over ptr to chainman MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It does not make sense to use a pointer, when a reference is more appropriate, especially given that nullptr has been ruled out. This is also allows to remove the CI workaround to avoid warnings: ``` C++ compiler .......................... GNU 13.0.0, /bin/x86_64-w64-mingw32-g++-posix ... /ci_container_base/src/test/blockmanager_tests.cpp: In member function ‘void blockmanager_tests::blockmanager_scan_unlink_already_pruned_files::test_method()’: /ci_container_base/src/test/blockmanager_tests.cpp:63:17: error: possibly dangling reference to a temporary [-Werror=dangling-reference] 63 | const auto& chainman = Assert(m_node.chainman); | ^~~~~~~~ In file included from /ci_container_base/src/streams.h:13, from /ci_container_base/src/dbwrapper.h:11, from /ci_container_base/src/node/blockstorage.h:10, from /ci_container_base/src/test/blockmanager_tests.cpp:8: /ci_container_base/src/util/check.h:116:49: note: the temporary was destroyed at the end of the full expression ‘inline_assertion_check&>(((blockmanager_tests::blockmanager_scan_unlink_already_pruned_files*)this)->blockmanager_tests::blockmanager_scan_unlink_already_pruned_files::.TestChain100Setup::.TestingSetup::.ChainTestingSetup::.BasicTestingSetup::m_node.node::NodeContext::chainman, std::source_location{(& *.Lsrc_loc27)}, std::basic_string_view(((const char*)"m_node.chainman")))’ 116 | #define Assert(val) inline_assertion_check(val, std::source_location::current(), #val) | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /ci_container_base/src/test/blockmanager_tests.cpp:63:28: note: in expansion of macro ‘Assert’ 63 | const auto& chainman = Assert(m_node.chainman); | ^~~~~~ cc1plus: all warnings being treated as errors gmake[2]: Leaving directory '/ci_container_base/ci/scratch/build-x86_64-w64-mingw32' gmake[2]: *** [src/test/CMakeFiles/test_bitcoin.dir/build.make:382: src/test/CMakeFiles/test_bitcoin.dir/blockmanager_tests.cpp.obj] Error 1 gmake[1]: *** [CMakeFiles/Makefile2:1810: src/test/CMakeFiles/test_bitcoin.dir/all] Error 2 gmake[1]: Leaving directory '/ci_container_base/ci/scratch/build-x86_64-w64-mingw32' gmake: *** [Makefile:146: all] Error 2 ``` This false-positive warning is also fixed in later GCC versions. See also https://godbolt.org/z/fjc6be65M --- ci/test/00_setup_env_arm.sh | 5 +---- ci/test/00_setup_env_win64.sh | 4 +--- src/test/blockmanager_tests.cpp | 20 ++++++++++---------- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/ci/test/00_setup_env_arm.sh b/ci/test/00_setup_env_arm.sh index d5b2bb4bf08..12ec71b8df5 100755 --- a/ci/test/00_setup_env_arm.sh +++ b/ci/test/00_setup_env_arm.sh @@ -19,10 +19,7 @@ export GOAL="install" export CI_LIMIT_STACK_SIZE=1 # -Wno-psabi is to disable ABI warnings: "note: parameter passing for argument of type ... changed in GCC 7.1" # This could be removed once the ABI change warning does not show up by default -# -# -Wno-error=dangling-reference helps to work around a GCC 13.1 false-positive, -# fixed in later versions. export BITCOIN_CONFIG=" \ -DREDUCE_EXPORTS=ON \ - -DCMAKE_CXX_FLAGS='-Wno-psabi -Wno-error=dangling-reference -Wno-error=maybe-uninitialized' \ + -DCMAKE_CXX_FLAGS='-Wno-psabi -Wno-error=maybe-uninitialized' \ " diff --git a/ci/test/00_setup_env_win64.sh b/ci/test/00_setup_env_win64.sh index 06134457867..110db1a8f1e 100755 --- a/ci/test/00_setup_env_win64.sh +++ b/ci/test/00_setup_env_win64.sh @@ -13,8 +13,6 @@ export PACKAGES="g++-mingw-w64-x86-64-posix nsis" export RUN_UNIT_TESTS=false export RUN_FUNCTIONAL_TESTS=false export GOAL="deploy" -# -Wno-error=dangling-reference helps to work around a GCC 13.1 false-positive, -# fixed in later versions. export BITCOIN_CONFIG="-DREDUCE_EXPORTS=ON -DBUILD_GUI_TESTS=OFF -DBUILD_KERNEL_LIB=ON -DBUILD_KERNEL_TEST=ON \ - -DCMAKE_CXX_FLAGS='-Wno-error=dangling-reference -Wno-error=maybe-uninitialized' \ + -DCMAKE_CXX_FLAGS='-Wno-error=maybe-uninitialized' \ " diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index f06665d3055..e6d5f153030 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -60,16 +60,16 @@ BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain100Setup) { // Cap last block file size, and mine new block in a new block file. - const auto& chainman = Assert(m_node.chainman); - auto& blockman = chainman->m_blockman; - const CBlockIndex* old_tip{WITH_LOCK(chainman->GetMutex(), return chainman->ActiveChain().Tip())}; - WITH_LOCK(chainman->GetMutex(), blockman.GetBlockFileInfo(old_tip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE); + auto& chainman{*Assert(m_node.chainman)}; + auto& blockman{chainman.m_blockman}; + const CBlockIndex* old_tip{WITH_LOCK(chainman.GetMutex(), return chainman.ActiveChain().Tip())}; + WITH_LOCK(chainman.GetMutex(), blockman.GetBlockFileInfo(old_tip->GetBlockPos().nFile)->nSize = MAX_BLOCKFILE_SIZE); CreateAndProcessBlock({}, GetScriptForRawPubKey(coinbaseKey.GetPubKey())); // Prune the older block file, but don't unlink it int file_number; { - LOCK(chainman->GetMutex()); + LOCK(chainman.GetMutex()); file_number = old_tip->GetBlockPos().nFile; blockman.PruneOneBlockFile(file_number); } @@ -78,22 +78,22 @@ BOOST_FIXTURE_TEST_CASE(blockmanager_scan_unlink_already_pruned_files, TestChain // Check that the file is not unlinked after ScanAndUnlinkAlreadyPrunedFiles // if m_have_pruned is not yet set - WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); + WITH_LOCK(chainman.GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); BOOST_CHECK(!blockman.OpenBlockFile(pos, true).IsNull()); // Check that the file is unlinked after ScanAndUnlinkAlreadyPrunedFiles // once m_have_pruned is set blockman.m_have_pruned = true; - WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); + WITH_LOCK(chainman.GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); BOOST_CHECK(blockman.OpenBlockFile(pos, true).IsNull()); // Check that calling with already pruned files doesn't cause an error - WITH_LOCK(chainman->GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); + WITH_LOCK(chainman.GetMutex(), blockman.ScanAndUnlinkAlreadyPrunedFiles()); // Check that the new tip file has not been removed - const CBlockIndex* new_tip{WITH_LOCK(chainman->GetMutex(), return chainman->ActiveChain().Tip())}; + const CBlockIndex* new_tip{WITH_LOCK(chainman.GetMutex(), return chainman.ActiveChain().Tip())}; BOOST_CHECK_NE(old_tip, new_tip); - const int new_file_number{WITH_LOCK(chainman->GetMutex(), return new_tip->GetBlockPos().nFile)}; + const int new_file_number{WITH_LOCK(chainman.GetMutex(), return new_tip->GetBlockPos().nFile)}; const FlatFilePos new_pos(new_file_number, 0); BOOST_CHECK(!blockman.OpenBlockFile(new_pos, true).IsNull()); } From 7a4901c9029687ad2b37f6d929d4c8fe96c15db3 Mon Sep 17 00:00:00 2001 From: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> Date: Mon, 3 Nov 2025 21:30:02 +0000 Subject: [PATCH 2/2] test, refactor: Fix `-Warray-bounds` warning --- src/test/net_tests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/net_tests.cpp b/src/test/net_tests.cpp index 711b067ad26..a2ff9a10993 100644 --- a/src/test/net_tests.cpp +++ b/src/test/net_tests.cpp @@ -1301,7 +1301,7 @@ public: { // Construct contents consisting of 0x00 + 12-byte message type + payload. std::vector contents(1 + CMessageHeader::MESSAGE_TYPE_SIZE + payload.size()); - std::copy(mtype.begin(), mtype.end(), reinterpret_cast(contents.data() + 1)); + std::copy(mtype.begin(), mtype.end(), contents.begin() + 1); std::copy(payload.begin(), payload.end(), contents.begin() + 1 + CMessageHeader::MESSAGE_TYPE_SIZE); // Send a packet with that as contents. SendPacket(contents);