Avoid implicit-integer-sign-change in VerifyLoadedChainstate #24403

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2202-intB changing 4 files +28 −15

MarcoFalke commented at 9:32 AM on February 21, 2022: member

This happens when checking all blocks (-1).

To test:

./configure CC=clang CXX=clang++ --with-sanitizers=undefined,integer
make
UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" ./test/functional/rpc_blockchain.py

Avoid implicit-integer-sign-change in VerifyLoadedChainstate fa462ea787
MarcoFalke added the label Refactoring on Feb 21, 2022
MarcoFalke cross-referenced this on Feb 21, 2022 from issue rpc: Fix implicit-integer-sign-change in verifychain by MarcoFalke
Fixup style of VerifyDB fa7991601c
theStack approved
theStack commented at 12:10 AM on February 23, 2022: contributor

Code-review ACK fa7991601c93761bc12ef33b672a927d48a95569

in src/node/chainstate.cpp:133 in fa7991601c

 128 | @@ -129,8 +129,8 @@ std::optional<ChainstateLoadVerifyError> VerifyLoadedChainstate(ChainstateManage
 129 |                                                                  bool fReset,
 130 |                                                                  bool fReindexChainState,
 131 |                                                                  const Consensus::Params& consensus_params,
 132 | -                                                                unsigned int check_blocks,
 133 | -                                                                unsigned int check_level,
 134 | +                                                                int check_blocks,
 135 | +                                                                int check_level,

benthecarman commented at 6:18 AM on February 23, 2022:

should assert(check_blocks >= 0) and assert(check_level >= 0) be added?

MarcoFalke commented at 7:22 AM on February 23, 2022:

No, this will crash the test

brunoerg approved
brunoerg commented at 12:16 AM on February 24, 2022: contributor

crACK fa7991601c93761bc12ef33b672a927d48a95569
MarcoFalke merged this on Feb 28, 2022
MarcoFalke closed this on Feb 28, 2022
MarcoFalke deleted the branch on Feb 28, 2022
sidhujag referenced this in commit 089d5bfbbb on Mar 1, 2022
bitcoin locked this on Feb 28, 2023

Contributors

Labels

Linked (view graph)

#24347 rpc: Fix implicit-integer-sign-change in verifychain