Flag relevant Sock methods with [[nodiscard]] to avoid issues like the one fixed in #21631.
net: flag relevant Sock methods with [[nodiscard]] #21659
pull vasild wants to merge 1 commits into bitcoin:master from vasild:Sock_nodiscard changing 4 files +21 −18-
vasild commented at 1:51 PM on April 12, 2021: contributor
- vasild cross-referenced this on Apr 12, 2021 from issue fuzz: split FuzzedSock interface and implementation by vasild
- vasild cross-referenced this on Apr 12, 2021 from issue i2p: always check the return value of Sock::Wait() by vasild
-
vasild commented at 1:58 PM on April 12, 2021: contributor
Having two
Wait()methods - one flagged with[[nodiscard]]with 3 arguments and one without[[nodiscard]]with 2 arguments seems like too much new lines of code.I am ~0 on this. Taking the dump/KISS approach for now - one
[[nodiscard]] Wait()and cast tovoidthe callers that don't check the return value. -
practicalswift commented at 2:19 PM on April 12, 2021: contributor
Concept ACK
Rationale: Having the
[[nodiscard]]annotation onWaitwould have saved us from one uninitialized read historically. That's enough empirical evidence to convince me about the need for[[nodiscard]]here.Thanks for addressing this!
- DrahtBot added the label P2P on Apr 12, 2021
- DrahtBot added the label Utils/log/libs on Apr 12, 2021
- fanquake added the label Needs rebase on Apr 13, 2021
-
net: flag relevant Sock methods with [[nodiscard]] e286cd0d7b
- vasild force-pushed on Apr 13, 2021
-
- vasild marked this as ready for review on Apr 13, 2021
- DrahtBot removed the label Needs rebase on Apr 13, 2021
-
practicalswift commented at 6:29 AM on April 14, 2021: contributor
cr ACK e286cd0d7b4e12c8efe5e7ac3066a100e0ba2c0a: the only changes made are additions of
[[nodiscard]]and(void)where appropriate - DrahtBot cross-referenced this on May 7, 2021 from issue Make all networking code mockable by vasild
-
DrahtBot commented at 11:28 PM on May 7, 2021: contributor
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--174a7506f384e20aa4161008e828411d-->
Conflicts
Reviewers, this pull request conflicts with the following ones:
- #21878 (Make all networking code mockable by vasild)
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.
-
laanwj commented at 1:08 PM on May 19, 2021: member
Code review ACK e286cd0d7b4e12c8efe5e7ac3066a100e0ba2c0a
- laanwj merged this on May 19, 2021
- laanwj closed this on May 19, 2021
- vasild deleted the branch on May 19, 2021
- sidhujag referenced this in commit 6ae83253d2 on May 19, 2021
- vasild cross-referenced this on May 20, 2021 from issue refactor: wrap accept() and extend usage of Sock by vasild
- gwillen referenced this in commit ba0573cbe6 on Jun 1, 2022
- bitcoin locked this on Aug 18, 2022
