← Watch feed
test: [refactor] Use reference over ptr to chainman
Public commit record
100/100 · Strong
What the developer wrote
Authored by MarcoFalke
test: [refactor] Use reference over ptr to chainman
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<true, std::unique_ptr<ChainstateManager>&>(((blockmanager_tests::blockmanager_scan_unlink_already_pruned_files*)this)->blockmanager_tests::blockmanager_scan_unlink_already_pruned_files::<anonymous>.TestChain100Setup::<anonymous>.TestingSetup::<anonymous>.ChainTestingSetup::<anonymous>.BasicTestingSetup::m_node.node::NodeContext::chainman, std::source_location{(& *.Lsrc_loc27)}, std::basic_string_view<char>(((const char*)"m_node.chainman")))’
116 | #define Assert(val) inline_assertion_check<true>(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
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Uses a recognizable type or scope✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This is a test-only code cleanup. It changes one test file to use a C++ reference instead of a pointer when accessing an internal object, and removes two compiler warning workarounds from the continuous-integration scripts. There is no change to the live Bitcoin Core software that users run, and no security fix or vulnerability is described.
Recommended action
No security action required. Treat as ordinary refactoring/test-hygiene change.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors src/test/blockmanager_tests.cpp to dereference Assert(m_node.chainman) once into a reference (auto& chainman{*Assert(m_node.chainman)}) rather than repeatedly using operator-> on the returned pointer. This silences a GCC 13 false-positive -Wdangling-reference warning. The CI workaround flags -Wno-error=dangling-reference are removed from ci/test/00_setup_env_arm.sh and ci/test/00_setup_env_win64.sh. No consensus, networking, wallet, or kernel code is modified.
Changed components
src/test/blockmanager_tests.cppci/test/00_setup_env_arm.shci/test/00_setup_env_win64.shInspect captured patch +12 / −17
diff --git a/ci/test/00_setup_env_arm.sh b/ci/test/00_setup_env_arm.sh
index d5b2bb4b..12ec71b8 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 06134457..110db1a8 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 f06665d3..e6d5f153 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());
}
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.