Break up CAddrMan's IMPLEMENT_SERIALIZE #4508
Conversation
|
Tested: waiting until a peers.dat write succeeds, restart bitcoin, see reading addresses succeeds. |
|
Can I have some ACKs, so I can update #4498 with it? @laanwj @gavinandresen @jgarzik @gmaxwell ? |
|
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. |
src/serialize.h
Outdated
There was a problem hiding this comment.
Need to initialize nSize to zero here...
|
Code review ACK except for init CSizeComputer::nSize comment. |
|
Fixed. |
|
Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4508_b069750d3f27c96a83700a08a2bb819902268857/ for binaries and test log. |
b069750 Break up CAddrMan's IMPLEMENT_SERIALIZE (Pieter Wuille)
4 participants
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.