Page MenuHomePhabricator

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

Authored by nakihito on Jun 4 2019, 21:48.

Details

Summary

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 5a6d00c6defc587e22c93e63029fdd538ce8858d ("Permit disconnection of outbound peers on bad/slow chains") which was merged into master four days ago.

Friendly ping @sdaftuar :-)

Tree-SHA512: 32e5631025b7ba7556a02c89d040fbe339c482a03f28d0dbc9871c699e1f8ac867619b89c5fd41fdcfcf0dc4d7c859295b26ccd988572145cc244261aec18ce9

Backport of Core PR11655
https://github.com/bitcoin/bitcoin/pull/11655

Test Plan
make check
test_runner.py

Diff Detail

Repository
rABC Bitcoin ABC
Branch
PR11655
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 6143
Build 10334: Bitcoin ABC Buildbot (legacy)
Build 10333: arc lint + arc unit

Event Timeline

Owners added a reviewer: Restricted Owners Package.Jun 4 2019, 21:48
This revision is now accepted and ready to land.Jun 4 2019, 22:01