It is never a nullptr, otherwise an assertion would fire in UpdatePeerStateForReceivedHeaders.
Passing a reference makes the code easier to read and less brittle.
refactor: Pass reference to last header, not pointer #26378
pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2210-p2p-less-pointers-🍟 changing 1 files +19 −20-
maflcko commented at 12:56 PM on October 24, 2022: member
-
fa579f3063
refactor: Pass reference to last header, not pointer
It is never a nullptr, otherwise an assertion would fire in UpdatePeerStateForReceivedHeaders. Passing a reference makes the code easier to read and less brittle.
- DrahtBot added the label Refactoring on Oct 24, 2022
-
DrahtBot commented at 10:24 PM on October 24, 2022: contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
Reviews
See the guideline for information on the review process.
Type Count Reviewers ACK 2 aureleoules, john-moffett Concept ACK 1 dergoegge <!--174a7506f384e20aa4161008e828411d-->
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #26140 (refactor: Move CNodeState members guarded by g_msgproc_mutex to Peer by dergoegge)
- #25268 (refactor: Introduce EvictionManager by dergoegge)
- #24571 (p2p: Prevent block index fingerprinting by sending additional getheaders messages by dergoegge)
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.
-
dergoegge commented at 10:52 AM on October 25, 2022: member
Concept ACK
-
john-moffett commented at 5:04 PM on November 1, 2022: contributor
ACK fa579f3
Code review and rebased on master. Built/tests/ran bitcoind. Agree with using
assertinstead ofAssume. The renamed parameters are clearer.I don't know the policy on unrelated spelling corrections, but maybe this is a good place to fix the log line at 2867: https://github.com/bitcoin/bitcoin/blob/fa579f306363ed03c1138121415b67b9b36b4d53/src/net_processing.cpp#L2867
I think it should read "to send to" rather than "to end to".
-
maflcko commented at 5:24 PM on November 1, 2022: member
"to end" is correct, meaning "to their chain tip"
- aureleoules approved
-
aureleoules commented at 3:07 PM on November 17, 2022: member
ACK fa579f306363ed03c1138121415b67b9b36b4d53
- maflcko merged this on Dec 8, 2022
- maflcko closed this on Dec 8, 2022
- maflcko deleted the branch on Dec 8, 2022
- sidhujag referenced this in commit fdda07dc55 on Dec 8, 2022
- bitcoin locked this on Dec 8, 2023
Contributors
Labels