Break up CAddrMan’s IMPLEMENT_SERIALIZE #4508

1. 
   sipa commented at 6:19 pm on July 10, 2014: member

   clang-format cannot deal with it, and, admittedly, it’s way too big anyway.

   In order not to duplicate the writing logic for computing the size, introduce a CSizeComputer in serialize.h: a minimal serializer stream implementation that only computes the number of bytes written. Due to inlining, this should be as efficient as the existing GetSerializeSize code.

   I’m thinking about getting rid of IMPLEMENT_SERIALIZE entirely, and replace it with one generic method that depending on template instantiation can do either read/write/size.

2. 
   sipa commented at 7:33 pm on July 10, 2014: member

   Tested: waiting until a peers.dat write succeeds, restart bitcoin, see reading addresses succeeds.
3. 
   sipa commented at 4:58 pm on July 16, 2014: member

   Can I have some ACKs, so I can update #4498 with it? @laanwj @gavinandresen @jgarzik @gmaxwell ?
4. 
   jgarzik commented at 6:21 pm on July 16, 2014: contributor

   quick it-works test. ACK.

   nit: a //comment or commit msg txt explaining lack of IMPLEMENT_SERIALIZE(). Your pull request description covered that, but PRs aren’t really part of the git history.

5. in src/serialize.h: in fded96db71 outdated

   837+
   838+public:
   839+    int nType;
   840+    int nVersion;
   841+
   842+    CSizeComputer(int nTypeIn, int nVersionIn) : nType(nTypeIn), nVersion(nVersionIn) {}
   

   ---

   ---

   

   gavinandresen commented at 6:23 pm on July 16, 2014:

   Need to initialize nSize to zero here…
6. 
   gavinandresen commented at 6:27 pm on July 16, 2014: contributor

   Code review ACK except for init CSizeComputer::nSize comment.
7. Break up CAddrMan's IMPLEMENT_SERIALIZE b069750d3f
8. 
   sipa commented at 7:17 pm on July 16, 2014: member

   Fixed.
9. 
   BitcoinPullTester commented at 7:41 pm on July 16, 2014: none

   Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4508_b069750d3f27c96a83700a08a2bb819902268857/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
10. sipa merged this on Jul 17, 2014
11. sipa closed this on Jul 17, 2014

    ---

12. sipa referenced this in commit fb203cab40 on Jul 17, 2014
13. MarcoFalke locked this on Sep 8, 2021

Contributors
sipa jgarzik gavinandresen BitcoinPullTester