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

e286cd0d7b4e12c8efe5e7ac3066a100e0ba2c0a net: flag relevant Sock methods with [[nodiscard]] (Vasil Dimov)

Pull request description:

  Flag relevant Sock methods with `[[nodiscard]]` to avoid issues like the one fixed in https://github.com/bitcoin/bitcoin/pull/21631.

ACKs for top commit:
  practicalswift:
    cr ACK e286cd0d7b4e12c8efe5e7ac3066a100e0ba2c0a: the only changes made are additions of `[[nodiscard]]` and `(void)` where appropriate
  laanwj:
    Code review ACK e286cd0d7b4e12c8efe5e7ac3066a100e0ba2c0a

Tree-SHA512: addc361968d24912bb625b42f4db557791556bf0ffad818252a89a32d76ac22758ec70f8282dcfbfd77eebec20a8e6bb7557c8ed08d50a58de95378c34955973

2 files changed, 18 insertions, 15 deletions

diff --git a/src/util/sock.cpp b/src/util/sock.cpp
index 0bc9795db3..b6c2a47434 100644
--- a/src/util/sock.cpp
+++ b/src/util/sock.cpp
@@ -179,7 +179,7 @@ void Sock::SendComplete(const std::string& data,
         // Wait for a short while (or the socket to become ready for sending) before retrying
         // if nothing was sent.
         const auto wait_time = std::min(deadline - now, std::chrono::milliseconds{MAX_WAIT_FOR_IO});
-        Wait(wait_time, SEND);
+        (void)Wait(wait_time, SEND);
     }
 }
 
@@ -262,7 +262,7 @@ std::string Sock::RecvUntilTerminator(uint8_t terminator,
 
         // Wait for a short while (or the socket to become ready for reading) before retrying.
         const auto wait_time = std::min(deadline - now, std::chrono::milliseconds{MAX_WAIT_FOR_IO});
-        Wait(wait_time, RECV);
+        (void)Wait(wait_time, RECV);
     }
 }
 
diff --git a/src/util/sock.h b/src/util/sock.h
index a4df7cd21b..59cc8c0b1d 100644
--- a/src/util/sock.h
+++ b/src/util/sock.h
@@ -64,7 +64,7 @@ public:
      * Get the value of the contained socket.
      * @return socket or INVALID_SOCKET if empty
      */
-    virtual SOCKET Get() const;
+    [[nodiscard]] virtual SOCKET Get() const;
 
     /**
      * Get the value of the contained socket and drop ownership. It will not be closed by the
@@ -82,26 +82,29 @@ public:
      * send(2) wrapper. Equivalent to `send(this->Get(), data, len, flags);`. Code that uses this
      * wrapper can be unit tested if this method is overridden by a mock Sock implementation.
      */
-    virtual ssize_t Send(const void* data, size_t len, int flags) const;
+    [[nodiscard]] virtual ssize_t Send(const void* data, size_t len, int flags) const;
 
     /**
      * recv(2) wrapper. Equivalent to `recv(this->Get(), buf, len, flags);`. Code that uses this
      * wrapper can be unit tested if this method is overridden by a mock Sock implementation.
      */
-    virtual ssize_t Recv(void* buf, size_t len, int flags) const;
+    [[nodiscard]] virtual ssize_t Recv(void* buf, size_t len, int flags) const;
 
     /**
      * connect(2) wrapper. Equivalent to `connect(this->Get(), addr, addrlen)`. Code that uses this
      * wrapper can be unit tested if this method is overridden by a mock Sock implementation.
      */
-    virtual int Connect(const sockaddr* addr, socklen_t addr_len) const;
+    [[nodiscard]] virtual int Connect(const sockaddr* addr, socklen_t addr_len) const;
 
     /**
      * getsockopt(2) wrapper. Equivalent to
      * `getsockopt(this->Get(), level, opt_name, opt_val, opt_len)`. Code that uses this
      * wrapper can be unit tested if this method is overridden by a mock Sock implementation.
      */
-    virtual int GetSockOpt(int level, int opt_name, void* opt_val, socklen_t* opt_len) const;
+    [[nodiscard]] virtual int GetSockOpt(int level,
+                                         int opt_name,
+                                         void* opt_val,
+                                         socklen_t* opt_len) const;
 
     using Event = uint8_t;
 
@@ -124,9 +127,9 @@ public:
      * value of `true` and `occurred` being set to 0.
      * @return true on success and false otherwise
      */
-    virtual bool Wait(std::chrono::milliseconds timeout,
-                      Event requested,
-                      Event* occurred = nullptr) const;
+    [[nodiscard]] virtual bool Wait(std::chrono::milliseconds timeout,
+                                    Event requested,
+                                    Event* occurred = nullptr) const;
 
     /* Higher level, convenience, methods. These may throw. */
 
@@ -154,17 +157,17 @@ public:
      * @throws std::runtime_error if the operation cannot be completed. In this case some bytes may
      * have been consumed from the socket.
      */
-    virtual std::string RecvUntilTerminator(uint8_t terminator,
-                                            std::chrono::milliseconds timeout,
-                                            CThreadInterrupt& interrupt,
-                                            size_t max_data) const;
+    [[nodiscard]] virtual std::string RecvUntilTerminator(uint8_t terminator,
+                                                          std::chrono::milliseconds timeout,
+                                                          CThreadInterrupt& interrupt,
+                                                          size_t max_data) const;
 
     /**
      * Check if still connected.
      * @param[out] errmsg The error string, if the socket has been disconnected.
      * @return true if connected
      */
-    virtual bool IsConnected(std::string& errmsg) const;
+    [[nodiscard]] virtual bool IsConnected(std::string& errmsg) const;
 
 protected:
     /**