refactor: Use C++11 default member initializers #15109

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:Mf1901-ctorClean changing 15 files +35 −82

MarcoFalke commented at 11:02 AM on January 5, 2019: member
Changes:
- Remove unused constructors that leave some members uninitialized
- Remove manual initialization in each constructor and prefer C++11 default member initializers
This is not a stylistic change, but a change that avoids bugs such as:
- fix uninitialized read when stringifying an addrLocal #14728
- qt: Initialize members in WalletModel #12426
- net: correctly initialize nMinPingUsecTime #6636
- ...
MarcoFalke added the label Refactoring on Jan 5, 2019
MarcoFalke force-pushed on Jan 5, 2019
MarcoFalke force-pushed on Jan 5, 2019
MarcoFalke force-pushed on Jan 5, 2019
practicalswift commented at 2:23 PM on January 5, 2019: contributor
Concept ACK

From our developer notes:
Initialize all non-static class members where they are defined. If this is skipped for a good reason (i.e., optimization on the critical path), add an explicit comment about this

Rationale: Ensure determinism by avoiding accidental use of uninitialized values. Also, static analyzers balk about this. Initializing the members in the declaration makes it easy to spot uninitialized ones.

class A { uint32_t m_count{0}; }

fanquake commented at 2:58 PM on January 5, 2019: member

Concept ACK

On macOS:

  CXX      libbitcoin_server_a-pow.o
  CXX      libbitcoin_server_a-rest.o
In file included from miner.cpp:17:
./net.h:414:33: error: expected ';' at end of declaration list
    bool setBannedIsDirty{false} GUARDED_BY(cs_setBanned);
                                ^
                                ;
1 error generated.

Use C++11 default member initializers fa2510d5c1
MarcoFalke force-pushed on Jan 5, 2019
hebasto commented at 3:26 PM on January 5, 2019: member

Concept ACK.
DrahtBot commented at 4:40 PM on January 5, 2019: member


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #14605 (Return of the Banman by dongcarl)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
donaloconnor commented at 11:03 PM on January 5, 2019: contributor

utACK fa2510d5c1cdf9c2cd5cc9887302ced4378c7202
in src/wallet/rpcwallet.cpp:1029 in fa2510d5c1
```
1030 |      tallyitem()
1031 |      {
1032 | -        nAmount = 0;
1033 | -        nConf = std::numeric_limits<int>::max();
1034 | -        fIsWatchonly = false;
1035 |      }
```
Empact commented at 12:57 AM on January 6, 2019:

nit: could drop this constructor (as it's equivalent to the default)

laanwj commented at 12:54 PM on January 7, 2019:

Note that this code is going away anyway as it's part of the account system

in src/wallet/walletdb.cpp:167 in fa2510d5c1

 174 |      CWalletScanState() {
 175 | -        nKeys = nCKeys = nWatchKeys = nKeyMeta = m_unknown_records = 0;
 176 | -        fIsEncrypted = false;
 177 | -        fAnyUnordered = false;
 178 | -        nFileVersion = 0;
 179 |      }

Empact commented at 12:58 AM on January 6, 2019:

nit: could drop this constructor

Empact commented at 1:00 AM on January 6, 2019: member

utACK https://github.com/bitcoin/bitcoin/pull/15109/commits/fa2510d5c1cdf9c2cd5cc9887302ced4378c7202
practicalswift commented at 9:30 AM on January 6, 2019: contributor

utACK fa2510d5c1cdf9c2cd5cc9887302ced4378c7202 modulo @Empact:s nits
promag commented at 7:59 PM on January 6, 2019: member

Big concept ACK.
laanwj commented at 12:55 PM on January 7, 2019: member

utACK fa2510d5c1cdf9c2cd5cc9887302ced4378c7202 (checked that all the fields match)
fanquake commented at 3:14 PM on January 8, 2019: member

utACK fa2510d
laanwj merged this on Jan 9, 2019
laanwj closed this on Jan 9, 2019
laanwj referenced this in commit 3f12515199 on Jan 9, 2019
MarcoFalke deleted the branch on Jan 9, 2019
laanwj referenced this in commit 070eaf7fe5 on Jan 14, 2019
pravblockc referenced this in commit a7a5a1c067 on Aug 9, 2021
pravblockc referenced this in commit 8e21d90eed on Aug 11, 2021
pravblockc referenced this in commit 95d588e9b4 on Aug 11, 2021
5tefan referenced this in commit bfba8f2211 on Aug 12, 2021
MarcoFalke locked this on Dec 16, 2021

Contributors

Labels

Linked (view graph)

#6636 net: correctly initialize nMinPingUsecTime #12426 qt: Initialize members in WalletModel #14728 fix uninitialized read when stringifying an addrLocal #15144 [refactor] CNode: Use C++11 default member initializers

refactor: Use C++11 default member initializers #15109

Conflicts