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

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:m_chain_sync.m_work_header changing 1 files +1 −0

practicalswift commented at 2:36 PM on November 10, 2017: contributor
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 :-)

Explicitly state assumption that state.m_chain_sync.m_work_header != nullptr in ConsiderEviction

Static analyzer (and humans!) will see ...

```
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,
and thus flag `state.m_chain_sync.m_work_header->GetBlockHash().ToString()`
as a potential null pointer dereference.

This commit makes the tacit assumption (m_work_header != nullptr) explicit.

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

63c2d83e58

practicalswift force-pushed on Nov 10, 2017
practicalswift renamed this:
~~Explicitly state assumption that state.m_chain_sync.m_work_header != nullptr in ConsiderEviction(…)~~
net: Explicitly state assumption that state.m_chain_sync.m_work_header != nullptr in ConsiderEviction(…)
on Nov 10, 2017
sdaftuar commented at 3:06 PM on November 10, 2017: member

utACK
MarcoFalke added the label P2P on Nov 10, 2017
promag commented at 1:32 AM on November 11, 2017: member

utACK.

Nit, reword to shorter Assert state.m_chain_sync.m_work_header in ConsiderEviction.
practicalswift renamed this:
~~net: Explicitly state assumption that state.m_chain_sync.m_work_header != nullptr in ConsiderEviction(…)~~
net: Assert state.m_chain_sync.m_work_header in ConsiderEviction
on Nov 12, 2017
laanwj merged this on Nov 15, 2017
laanwj closed this on Nov 15, 2017
laanwj referenced this in commit aca77a4d58 on Nov 15, 2017
markblundeberg referenced this in commit e755a852db on Jun 5, 2019
jtoomim referenced this in commit 2ef3c27a30 on Jun 29, 2019
jonspock referenced this in commit bacd4127ed on Jul 4, 2019
jonspock referenced this in commit 2c1056d027 on Jul 4, 2019
proteanx referenced this in commit 4d52f0671d on Jul 5, 2019
jonspock referenced this in commit 0b4a16fa7a on Jul 9, 2019
PastaPastaPasta referenced this in commit 22fb3f11d1 on Jan 17, 2020
PastaPastaPasta referenced this in commit 33a163352f on Jan 22, 2020
PastaPastaPasta referenced this in commit 8fd40b4d67 on Jan 22, 2020
PastaPastaPasta referenced this in commit 33b5d9e5e8 on Jan 29, 2020
PastaPastaPasta referenced this in commit ee75012bc6 on Jan 29, 2020
PastaPastaPasta referenced this in commit 0dbfdad044 on Jan 29, 2020
ckti referenced this in commit 1ffcf20a6d on Mar 28, 2021
practicalswift deleted the branch on Apr 10, 2021
gades referenced this in commit de505328a9 on Feb 20, 2022
DrahtBot locked this on Aug 16, 2022

Contributors

Labels