Initialize nVersionDummy to zero in deserialization code #12767

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:initialize-nVersionDummy-to-zero changing 2 files +2 −2

practicalswift commented at 1:12 PM on March 23, 2018: contributor
Initialize nVersionDummy to zero in deserialization code.

Silences static analyzer warnings.

Hopefully uncontroversial – we're already initializing other variables to zero prior to passing them to ::Unserialize(…, VARINT(<variable>));:
```
$ git grep -B1 "::Unserialize.*VARINT"
src/coins.h-        uint32_t code = 0;
src/coins.h:        ::Unserialize(s, VARINT(code));
…
src/undo.h-        unsigned int nCode = 0;
src/undo.h:        ::Unserialize(s, VARINT(nCode));
…
```
Initialize nVersionDummy to zero in deserialization code
Silences static analyzer warnings.
b3b2103162
fanquake added the label Refactoring on Mar 23, 2018
hkjn commented at 2:46 PM on March 23, 2018: contributor

utACK b3b2103.

Seems straightforward / uncontroversial, but if it's not I look forward to learning why. :)
randolf approved
instagibbs commented at 3:57 PM on March 23, 2018: member

utACK https://github.com/bitcoin/bitcoin/pull/12767/commits/b3b21031620934f36b30822fb31435355ccbad0f
donaloconnor approved
donaloconnor commented at 8:27 PM on March 23, 2018: contributor

utACK https://github.com/bitcoin/bitcoin/commit/b3b21031620934f36b30822fb31435355ccbad0f
sipa commented at 7:57 PM on March 24, 2018: member

Why is this necessary? It seems the static analysis is wrong here. The practice of initializing variables whose value is never used is not necessarily good, as it hides reasoning errors about what should and shouldn't be used. Ideally I think we should explicitly choose to not initialize things when we believe they aren't being used.
laanwj commented at 9:50 PM on March 29, 2018: member

Agree with @sipa that this is unnecessary, and can even be harmful: what if a real problem is masked here?
practicalswift commented at 8:18 PM on April 2, 2018: contributor

Consensus NACK. Closing :-)
practicalswift closed this on Apr 2, 2018
practicalswift deleted the branch on Apr 10, 2021
DrahtBot locked this on Aug 18, 2022

Contributors

Labels