-
Notifications
You must be signed in to change notification settings - Fork 37.7k
net: Move SocketSendData lock annotation to header #20864
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Concept ACK. If you're moving things around in net.h to add lock annotations, might be worth moving Moving It would be nice if there were some easy way to review pointer-becomes-reference changes. |
Apart from
Thanks, done |
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fad2e1f
Nice tidy-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK fad2e1f
With the suggestions in the commit messages on how to best show the diffs, the review was quite straight-forward and painless. 👌
ACK fad2e1f |
Rebased, should be trivial to re-ACK with git range-diff or from scratch |
Can be reviewed with --color-moved=dimmed-zebra --patience
Also, add lock annotation to SendMessages Can be reviewed with "--word-diff-regex=."
utACK fa21068 |
Summary: ``` Can be reviewed with --color-moved=dimmed-zebra --patience ``` This is a move only change, no change in behavior. Partial backport of [[bitcoin/bitcoin#20864 | core#20864]]: bitcoin/bitcoin@fa0a717 Depends on D10879. Ref T1696. Test Plan: ninja all check-all Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Maniphest Tasks: T1696 Differential Revision: https://reviews.bitcoinabc.org/D10880
Summary: ``` Also, add lock annotation to SendMessages Can be reviewed with "--word-diff-regex=." ``` Completes backport of [[bitcoin/bitcoin#20864 | core#20864]]: bitcoin/bitcoin@fa21068 Depends on D10880. Ref T1696. Test Plan: ninja all check-all Reviewers: #bitcoin_abc, PiRK Reviewed By: #bitcoin_abc, PiRK Maniphest Tasks: T1696 Differential Revision: https://reviews.bitcoinabc.org/D10881
Lock annotations must be in the header, otherwise the will have limited or no effect