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

e286cd0 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 bitcoin#21631.

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

Tree-SHA512: addc361968d24912bb625b42f4db557791556bf0ffad818252a89a32d76ac22758ec70f8282dcfbfd77eebec20a8e6bb7557c8ed08d50a58de95378c34955973

Author: laanwj

src/test/fuzz/i2p.cpp

@@ -37,7 +37,7 @@ FUZZ_TARGET_INIT(i2p, initialize_i2p)
     if (sess.Listen(conn)) {
         if (sess.Accept(conn)) {
             try {
-                conn.sock->RecvUntilTerminator('\n', 10ms, interrupt, i2p::sam::MAX_MSG_SIZE);
+                (void)conn.sock->RecvUntilTerminator('\n', 10ms, interrupt, i2p::sam::MAX_MSG_SIZE);
             } catch (const std::runtime_error&) {
             }
         }

src/test/sock_tests.cpp

@@ -139,7 +139,7 @@ BOOST_AUTO_TEST_CASE(wait)
     Sock sock0(s[0]);
     Sock sock1(s[1]);
 
-    std::thread waiter([&sock0]() { sock0.Wait(24h, Sock::RECV); });
+    std::thread waiter([&sock0]() { (void)sock0.Wait(24h, Sock::RECV); });
 
     BOOST_REQUIRE_EQUAL(sock1.Send("a", 1, 0), 1);
 
@@ -162,7 +162,7 @@ BOOST_AUTO_TEST_CASE(recv_until_terminator_limit)
         // BOOST_CHECK_EXCEPTION() writes to some variables shared with the main thread which
         // creates a data race. So mimic it manually.
         try {
-            sock_recv.RecvUntilTerminator('\n', timeout, interrupt, max_data);
+            (void)sock_recv.RecvUntilTerminator('\n', timeout, interrupt, max_data);
         } catch (const std::runtime_error& e) {
             threw_as_expected = HasReason("too many bytes without a terminator")(e);
         }

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);
     }
 }
 

src/util/sock.h

@@ -64,7 +64,7 @@ class Sock
      * 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 @@ class Sock
      * 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 @@ class Sock
      * 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 @@ class Sock
      * @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:
     /**