rpc: add null assert #7493

pull dcousens wants to merge 1 commits into bitcoin:master from dcousens:patch-1 changing 1 files +3 −1

dcousens commented at 11:31 PM on February 9, 2016: contributor

reference #6393

Is that what you meant @laanwj ?
rpc: add null assert
reference #6393
0077cbf754
in src/rpc/blockchain.cpp:None in 0077cbf754
```
 669 | +        assert(block);
 670 | +        
 671 | +        while (block->pprev && (block->pprev->nStatus & BLOCK_HAVE_DATA))
 672 |              block = block->pprev;
 673 |  
 674 |          obj.push_back(Pair("pruneheight",        block->nHeight));
```
luke-jr commented at 11:47 PM on February 9, 2016:

I don't know the context of this PR, but it seems to me this should just be block?block->nHeight:0

dcousens commented at 1:44 AM on February 11, 2016:

@luke-jr I think it won't always be 0 since & BLOCK_HAVE_DATA would fail for the first pruned block, hence, the last "pruned" block.

luke-jr commented at 1:58 AM on February 11, 2016:

In that case, block won't be NULL... (I assume you're familiar with the trinary operator?)

dcousens commented at 2:22 AM on February 11, 2016:

Do you mean the ternary operator?

dcousens commented at 2:25 AM on February 11, 2016:

And yes, I understand the block won't be NULL... I'm not sure I understand what your point is?
luke-jr commented at 11:47 PM on February 9, 2016: member

Also, the commit message here sucks. Please explain what it's supposed to do and why...
laanwj commented at 10:22 AM on February 10, 2016: member

What I meant in #6393 was that when execution reaches the part of AppInit2() where the RPC server is unlocked (meaning SetRPCWarmupFinished() is called), the code has made sure that chainActive.Tip() is not null. It either has successfully indexed the genesis block, or exit with an error. See also #5243.

This means that checks (or assertions) for chainActive.Tip() are not necessary in the RPC code (or anywhere else if AppInit2 is finished). No need to add this. Luckily, because there are tons of places in the code where it is blindly assumed that the tip contains a block.
dcousens commented at 1:43 AM on February 11, 2016: contributor

Well, as was pointed out in #6393 the block && check is pointless given the immediate de-reference afterwards.
laanwj commented at 7:41 AM on February 11, 2016: member

But it doesn't really hurt either. I see no reason for changing this code.
laanwj closed this on Feb 11, 2016
dcousens deleted the branch on Feb 11, 2016
MarcoFalke locked this on Sep 8, 2021

Contributors

dcousens

luke-jr

laanwj

Linked (view graph)

#6393 Potential nullptr dereference in getblockchaininfo