Merge #11655: net: Assert state.m_chain_sync.m_work_header in ConsiderEviction

63c2d83 Explicitly state assumption that state.m_chain_sync.m_work_header != nullptr in ConsiderEviction (practicalswift)

Pull request description:

  Explicitly state assumption that `state.m_chain_sync.m_work_header != nullptr` in `ConsiderEviction(…)`.

  Static analyzer (and humans!) will see the null-check in ...

  ```
  else if (state.m_chain_sync.m_timeout == 0 || (state.m_chain_sync.m_work_header != nullptr && ...
  ```

  ... and infer that `state.m_chain_sync.m_work_header` might be set to `nullptr` when reaching `else if (state.m_chain_sync.m_timeout > 0 && time_in_seconds > state.m_chain_sync.m_timeout)` and thus flag `state.m_chain_sync.m_work_header->GetBlockHash().ToString()` as a potential null pointer dereference.

  This commit makes the tacit assumption of `state.m_chain_sync.m_work_header != nullptr` explicit.

  Code introduced in 5a6d00c6de ("Permit disconnection of outbound peers on bad/slow chains") which was merged into master four days ago.

  Friendly ping @sdaftuar :-)

Tree-SHA512: 32e5631025b7ba7556a02c89d040fbe339c482a03f28d0dbc9871c699e1f8ac867619b89c5fd41fdcfcf0dc4d7c859295b26ccd988572145cc244261aec18ce9
This commit is contained in:
Wladimir J. van der Laan 2017-11-15 13:55:03 +01:00
commit aca77a4d58
No known key found for this signature in database
GPG key ID: 1E4AED62986CD25D

View file

@ -3006,6 +3006,7 @@ void PeerLogicValidation::ConsiderEviction(CNode *pto, int64_t time_in_seconds)
LogPrintf("Disconnecting outbound peer %d for old chain, best known block = %s\n", pto->GetId(), state.pindexBestKnownBlock != nullptr ? state.pindexBestKnownBlock->GetBlockHash().ToString() : "<none>"); LogPrintf("Disconnecting outbound peer %d for old chain, best known block = %s\n", pto->GetId(), state.pindexBestKnownBlock != nullptr ? state.pindexBestKnownBlock->GetBlockHash().ToString() : "<none>");
pto->fDisconnect = true; pto->fDisconnect = true;
} else { } else {
assert(state.m_chain_sync.m_work_header);
LogPrint(BCLog::NET, "sending getheaders to outbound peer=%d to verify chain work (current best known block:%s, benchmark blockhash: %s)\n", pto->GetId(), state.pindexBestKnownBlock != nullptr ? state.pindexBestKnownBlock->GetBlockHash().ToString() : "<none>", state.m_chain_sync.m_work_header->GetBlockHash().ToString()); LogPrint(BCLog::NET, "sending getheaders to outbound peer=%d to verify chain work (current best known block:%s, benchmark blockhash: %s)\n", pto->GetId(), state.pindexBestKnownBlock != nullptr ? state.pindexBestKnownBlock->GetBlockHash().ToString() : "<none>", state.m_chain_sync.m_work_header->GetBlockHash().ToString());
connman->PushMessage(pto, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(state.m_chain_sync.m_work_header->pprev), uint256())); connman->PushMessage(pto, msgMaker.Make(NetMsgType::GETHEADERS, chainActive.GetLocator(state.m_chain_sync.m_work_header->pprev), uint256()));
state.m_chain_sync.m_sent_getheaders = true; state.m_chain_sync.m_sent_getheaders = true;