[refactor] CNode: Use C++11 default member initializers #15144

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:Mf1901-ctorClean changing 2 files +45 −87

MarcoFalke commented at 8:00 PM on January 10, 2019: member
The second and last change on this topic (c.f. #15109). Split up because the diff would otherwise interleave, making review harder than necessary.

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
- ...
Use C++11 default member initializers fac2f5ecae
MarcoFalke added the label Refactoring on Jan 10, 2019
MarcoFalke added the label P2P on Jan 10, 2019
DrahtBot commented at 8:08 PM on January 10, 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:
- #14929 (net: Allow connections from misbehavior banned peers by gmaxwell)
- #14897 (randomize GETDATA(tx) request order and introduce bias toward outbound by naumenkogs)
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.
practicalswift commented at 8:15 PM on January 10, 2019: contributor

Concept ACK. Will review.
in src/net.h:647 in fac2f5ecae
```
 646 | -    size_t nSendSize; // total size of all vSendMsg entries
 647 | -    size_t nSendOffset; // offset inside the first vSendMsg already sent
 648 | -    uint64_t nSendBytes GUARDED_BY(cs_vSend);
 649 | +    size_t nSendSize{0}; // total size of all vSendMsg entries
 650 | +    size_t nSendOffset{0}; // offset inside the first vSendMsg already sent
 651 | +    uint64_t nSendBytes GUARDED_BY(cs_vSend){0};
```
practicalswift commented at 8:19 PM on January 10, 2019:

I'm all for brace initialisation in the general case, but in this case what about using ...

uint64_t nSendBytes = 0 GUARDED_BY(cs_vSend);

... to make it clear that 0 has nothing to do with GUARDED_BY(cs_vSend)?

Applies for the other member variables with GUARDED_BY(…):s.
promag commented at 12:46 AM on January 11, 2019:

I prefer consistency, and maybe a space around the annotation would look better:

uint64_t nSendBytes GUARDED_BY(cs_vSend) {0};
practicalswift commented at 8:28 AM on January 11, 2019:

@promag Yes, space is better.

MarcoFalke commented at 12:25 PM on January 11, 2019:

According to clang-format there should be no space

practicalswift commented at 1:25 PM on January 11, 2019:

Oh, TIL! Then we'll simply go with what clang-format prefers.

in src/net.h:751 in fac2f5ecae

 755 |      // Last measured round-trip time.
 756 | -    std::atomic<int64_t> nPingUsecTime;
 757 | +    std::atomic<int64_t> nPingUsecTime{0};
 758 |      // Best measured round-trip time.
 759 | -    std::atomic<int64_t> nMinPingUsecTime;
 760 | +    std::atomic<int64_t> nMinPingUsecTime{std::numeric_limits<int64_t>::max()};

Empact commented at 10:17 PM on January 10, 2019:

#include <limits>

promag commented at 12:50 AM on January 11, 2019: member
ACK fac2f5e:
- initialization values are the same
- comments are moved
- some styles fixes
- annotations are unchanged
sipa commented at 6:06 PM on January 11, 2019: member

utACK fac2f5ecae96dd11057977ce988501e18bb162c6

in src/net.cpp:2814 in fac2f5ecae

2819 | -    nSendBytes = 0;
2820 | -    nRecvBytes = 0;
2821 | -    nTimeOffset = 0;
2822 |      addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn;
2823 | -    nVersion = 0;
2824 |      strSubVer = "";

practicalswift commented at 6:21 PM on January 11, 2019:

Remove this redundant line? :-)

Empact commented at 8:11 AM on January 12, 2019: member

utACK https://github.com/bitcoin/bitcoin/commit/fac2f5ecae96dd11057977ce988501e18bb162c6
laanwj commented at 1:48 PM on January 14, 2019: member

utACK fac2f5ecae96dd11057977ce988501e18bb162c6
laanwj merged this on Jan 14, 2019
laanwj closed this on Jan 14, 2019
laanwj referenced this in commit 070eaf7fe5 on Jan 14, 2019
MarcoFalke deleted the branch on Jan 14, 2019
vijaydasmp referenced this in commit ba08769bf9 on Aug 22, 2021
vijaydasmp referenced this in commit b8a07d97a5 on Aug 22, 2021
vijaydasmp referenced this in commit 4421bb145c on Aug 22, 2021
vijaydasmp referenced this in commit d7719d5439 on Aug 29, 2021
vijaydasmp referenced this in commit ab10fa3710 on Aug 29, 2021
vijaydasmp referenced this in commit 7c0482f47e on Aug 30, 2021
PastaPastaPasta referenced this in commit 07cf6cdf48 on Aug 31, 2021
DrahtBot 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 #15109 refactor: Use C++11 default member initializers

[refactor] CNode: Use C++11 default member initializers #15144

Conflicts