net: flag relevant Sock methods with [[nodiscard]] #21659

1. 
   vasild commented at 1:51 pm on April 12, 2021: member

   Flag relevant Sock methods with [[nodiscard]] to avoid issues like the one fixed in #21631.
2. 
   vasild commented at 1:58 pm on April 12, 2021: member

   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 to void the callers that don’t check the return value.

   cc @practicalswift

3. 
   practicalswift commented at 2:19 pm on April 12, 2021: contributor

   Concept ACK

   Rationale: Having the [[nodiscard]] annotation on Wait would 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!

4. DrahtBot added the label P2P on Apr 12, 2021
5. DrahtBot added the label Utils/log/libs on Apr 12, 2021
6. fanquake added the label Needs rebase on Apr 13, 2021
7. net: flag relevant Sock methods with [[nodiscard]] e286cd0d7b
8. vasild force-pushed on Apr 13, 2021
9. 
   vasild commented at 3:27 pm on April 13, 2021: member

   51878b307...e286cd0d7: rebased, now #21631 is merged and I removed it from this PR
10. vasild marked this as ready for review on Apr 13, 2021
11. DrahtBot removed the label Needs rebase on Apr 13, 2021
12. 
    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
13. 
    DrahtBot commented at 11:28 pm on May 7, 2021: member

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    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.

14. 
    laanwj commented at 1:08 pm on May 19, 2021: member

    Code review ACK e286cd0d7b4e12c8efe5e7ac3066a100e0ba2c0a
15. laanwj merged this on May 19, 2021
16. laanwj closed this on May 19, 2021

    ---

17. vasild deleted the branch on May 19, 2021
18. sidhujag referenced this in commit 6ae83253d2 on May 19, 2021
19. gwillen referenced this in commit ba0573cbe6 on Jun 1, 2022
20. DrahtBot locked this on Aug 18, 2022

Contributors
vasild practicalswift DrahtBot laanwj

Labels
P2P Utils/log/libs