Prior to this commit there was an implicit assumption that the following code path would always be taken ...
if (pindexPrev != chainActive.Tip() ||
(mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - nStart > 5))... otherwise pblock would end up being NULL, and given ...
pblock->nNonce = 0;... we would have a NULL pointer dereference.
This commit adds an assertion which removes the possibility of a NULL pointer dereference.
Prior to this commit there was an implicit assumption that the following
code path would always be taken ...
if (pindexPrev != chainActive.Tip() ||
(mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - nStart > 5))
... otherwise pblock would end up being NULL, and given ...
pblock->nNonce = 0;
... we would have a NULL pointer dereference.
This commit adds an assertion which removes the possibility of a NULL
pointer dereference.543 | @@ -544,6 +544,7 @@ UniValue getblocktemplate(const JSONRPCRequest& request) 544 | pindexPrev = pindexPrevNew; 545 | } 546 | CBlock* pblock = &pblocktemplate->block; // pointer for convenience 547 | + assert(pblock);
It's impossible for pblock to be NULL here. The dereference is one line above. And pindexPrev initialises to NULL, so the above initialisation will indeed always occur.
The dereference I'm trying to avoid is not on the line above, but a couple of lines below - see here (pblock->nNonce = 0;).
Is your reading that if-branch on lines 526-546 will always be taken?
If that branch is not taken then pblock would be NULL, right? :-)
Sorry. After re-reading the comment and the code I think I understand - your reading is that the if-branch is guaranteed to be taken on the first call, right? :-)
I've now re-read the code and now my reading is that assuming the if-branch is taken on the first call then there is no possibility of a NULL pointer dereference.
The question then boils down to if pindexPrev != chainActive.Tip() || (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLast && GetTime() - nStart > 5) is guaranteed to be true on the first call :-)
It is guaranteed to be true on the first call.
But even if it wasn't, pblock would still never be NULL. It is always assigned to &pblocktemplate->block, which might be NULL with some platforms, but is not guaranteed to be, and if not would pass your assert yet still be wrong.
OK, thanks for reviewing. Closing PR.