highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-06-15 00:59:38 +02:00
Code Issues Packages Projects Releases Wiki Activity

test: [refactor] Use reference over ptr to chainman

Browse Source 
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
This commit is contained in:
MarcoFalke
2025-11-10 12:50:15 +01:00
parent 490cb056f6
commit faf2759c8c
3 changed files with 12 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
ci/test/00_setup_env_arm.sh
View File

@@ -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' \
"

4
ci/test/00_setup_env_win64.sh
View File

@@ -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' \
"

20
src/test/blockmanager_tests.cpp
View File

@@ -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());
}