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
  1. MarcoFalke commented at 9:43 pm on May 19, 2020: member
    This change removes 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.
  2. 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. As nTime 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.
  3. DrahtBot added the label Consensus on May 19, 2020
  4. DrahtBot added the label P2P on May 19, 2020
  5. DrahtBot added the label RPC/REST/ZMQ on May 19, 2020
  6. DrahtBot added the label Tests on May 19, 2020
  7. DrahtBot added the label TX fees and policy on May 19, 2020
  8. DrahtBot added the label UTXO Db and Indexes on May 19, 2020
  9. DrahtBot commented at 7:20 am on May 20, 2020: member

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

    Conflicts

    No conflicts as of last run.

  10. practicalswift commented at 8:52 am on May 20, 2020: contributor

    Concept ACK

    AFAICT the suggested change is fully in line with with the recommendations in the C++ Core Guidelines (Stroustrup & Sutter):

  11. MarcoFalke removed the label Consensus on May 20, 2020
  12. MarcoFalke removed the label RPC/REST/ZMQ on May 20, 2020
  13. MarcoFalke removed the label TX fees and policy on May 20, 2020
  14. MarcoFalke removed the label Tests on May 20, 2020
  15. MarcoFalke removed the label UTXO Db and Indexes on May 20, 2020
  16. MarcoFalke added the label Refactoring on May 20, 2020
  17. MarcoFalke force-pushed on May 20, 2020
  18. net: Use C++11 member initialization in protocol fa8bbb1368
  19. MarcoFalke force-pushed on May 20, 2020
  20. MarcoFalke marked this as ready for review on May 20, 2020
  21. in 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’t unsigned int be the value to test for equivalence?

    sipa commented at 10:03 pm on May 20, 2020:
    unsigned is a shorthand for unsigned 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.
  22. laanwj commented at 3:42 pm on May 21, 2020: member
    ACK fa8bbb1368be0f3fd9cc4446aead3f4c2188a4ab Nice cleanup. Checked that nTime’s (100000000) and nServices (NODE_NONE) initial value stays the same in all cases.
  23. laanwj merged this on May 21, 2020
  24. laanwj closed this on May 21, 2020

  25. MarcoFalke deleted the branch on May 21, 2020
  26. sidhujag referenced this in commit 2864507312 on May 21, 2020
  27. deadalnix referenced this in commit fcf518ba31 on Feb 9, 2021
  28. random-zebra referenced this in commit b4751e10ce on Aug 11, 2021
  29. DrahtBot 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 site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me