Init
from the constructors and instead uses C++11 member initialization. This removes a bunch of boilerplate, makes the code easier to read. Also, C++11 member initialization avoids accidental uninitialized members.
net: Use C++11 member initialization in protocol #19020
pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2005-netCxx11MemberInit changing 3 files +8 −26-
MarcoFalke commented at 9:43 pm on May 19, 2020: memberThis change removes
-
in src/protocol.h:339 in eac91b0376 outdated
336+ CAddress() : CService{} {}; 337+ explicit CAddress(CService ipIn, ServiceFlags nServicesIn) : CService{ipIn}, nServices{nServicesIn} {}; 338 339 SERIALIZE_METHODS(CAddress, obj) 340 { 341- SER_READ(obj, obj.Init());
sipa commented at 9:48 pm on May 19, 2020:This may be fine here, but it’s not obvious. When deserializing, the receiving object is expected to be overwritten completely. AsnTime
is only conditionally deserialized, the alternative would need a reset for that value.
MarcoFalke commented at 12:37 pm on May 20, 2020:Thanks, fixed.DrahtBot added the label Consensus on May 19, 2020DrahtBot added the label P2P on May 19, 2020DrahtBot added the label RPC/REST/ZMQ on May 19, 2020DrahtBot added the label Tests on May 19, 2020DrahtBot added the label TX fees and policy on May 19, 2020DrahtBot added the label UTXO Db and Indexes on May 19, 2020DrahtBot commented at 7:20 am on May 20, 2020: memberThe following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Conflicts
No conflicts as of last run.
practicalswift commented at 8:52 am on May 20, 2020: contributorConcept ACK
AFAICT the suggested change is fully in line with with the recommendations in the C++ Core Guidelines (Stroustrup & Sutter):
- C.41: A constructor should create a fully initialized object: “A constructor establishes the invariant for a class. A user of a class should be able to assume that a constructed object is usable.”
- C.45: Don’t define a default constructor that only initializes data members; use in-class member initializers instead: “Using in-class member initializers lets the compiler generate the function for you. The compiler-generated function can be more efficient.”
- C.48: Prefer in-class initializers to member initializers in constructors for constant initializers: “Makes it explicit that the same value is expected to be used in all constructors. Avoids repetition. Avoids maintenance problems. It leads to the shortest and most efficient code.”
- C.49: Prefer initialization to assignment in constructors: “An initialization explicitly states that initialization, rather than assignment, is done and can be more elegant and efficient. Prevents “use before set” errors.”
- ES.20: Always initialize an object: “Avoid used-before-set errors and their associated undefined behavior. Avoid problems with comprehension of complex initialization. Simplify refactoring.”
- ES.23: Prefer the {}-initializer syntax: “Prefer {}. The rules for {} initialization are simpler, more general, less ambiguous, and safer than for other forms of initialization. Use = only when you are sure that there can be no narrowing conversions. For built-in arithmetic types, use = only with auto. Avoid () initialization, which allows parsing ambiguities.”
MarcoFalke removed the label Consensus on May 20, 2020MarcoFalke removed the label RPC/REST/ZMQ on May 20, 2020MarcoFalke removed the label TX fees and policy on May 20, 2020MarcoFalke removed the label Tests on May 20, 2020MarcoFalke removed the label UTXO Db and Indexes on May 20, 2020MarcoFalke added the label Refactoring on May 20, 2020MarcoFalke force-pushed on May 20, 2020net: Use C++11 member initialization in protocol fa8bbb1368MarcoFalke force-pushed on May 20, 2020MarcoFalke marked this as ready for review on May 20, 2020in src/compat/assumptions.h:53 in fa8bbb1368
49@@ -50,6 +50,7 @@ static_assert(sizeof(double) == 8, "64-bit double assumed"); 50 // code. 51 static_assert(sizeof(short) == 2, "16-bit short assumed"); 52 static_assert(sizeof(int) == 4, "32-bit int assumed"); 53+static_assert(sizeof(unsigned) == 4, "32-bit unsigned assumed");
Empact commented at 9:50 pm on May 20, 2020:Wouldn’tunsigned int
be the value to test for equivalence?
sipa commented at 10:03 pm on May 20, 2020:unsigned
is a shorthand forunsigned int
.
MarcoFalke commented at 10:57 am on May 21, 2020:I think this check is redundant anyway, because prepending a type with unsigned shouldn’t change it’s bitlength. But I didn’t feel like looking that up and adding the check has no downsides.laanwj commented at 3:42 pm on May 21, 2020: memberACK fa8bbb1368be0f3fd9cc4446aead3f4c2188a4ab Nice cleanup. Checked thatnTime
’s (100000000) andnServices
(NODE_NONE
) initial value stays the same in all cases.laanwj merged this on May 21, 2020laanwj closed this on May 21, 2020
MarcoFalke deleted the branch on May 21, 2020sidhujag referenced this in commit 2864507312 on May 21, 2020deadalnix referenced this in commit fcf518ba31 on Feb 9, 2021random-zebra referenced this in commit b4751e10ce on Aug 11, 2021DrahtBot locked this on Feb 15, 2022
github-metadata-mirror
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-11 06:12 UTC
This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-12-11 06:12 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me
More mirrored repositories can be found on mirror.b10c.me