Merge #10865: Move CloseSocket out of SetSocketNonBlocking and pass socket as const reference

05e023f Move CloseSocket out of SetSocketNonBlocking and pass SOCKET by const reference in SetSocket* functions (Dag Robole)

Pull request description:

  Rationale:

  Readability, SetSocketNonBlocking does what it says on the tin.

  Consistency, More consistent with the rest of the API in this unit.

  Reusability, SetSocketNonBlocking can also be used by clients that may not want to close the socket on failure.

  This also moves the responsibility of closing the socket back to the caller that opened it, which in general should know better how and when to close it.

Tree-SHA512: 85027137f1b626e2b636549ee38cc757a587adcf464c84be6e65ca16e3b75d7ed1a1b21dd70dbe34c7c5d599af39e53b89932dfe3c74f91a22341ff3af5ea80a

4 files changed, 10 insertions, 9 deletions

diff --git a/src/compat.h b/src/compat.h
index e76ab94c82..e022659c01 100644
--- a/src/compat.h
+++ b/src/compat.h
@@ -76,7 +76,7 @@ typedef unsigned int SOCKET;
 size_t strnlen( const char *start, size_t max_len);
 #endif // HAVE_DECL_STRNLEN
 
-bool static inline IsSelectableSocket(SOCKET s) {
+bool static inline IsSelectableSocket(const SOCKET& s) {
 #ifdef WIN32
     return true;
 #else
diff --git a/src/net.cpp b/src/net.cpp
index 5bf3af7ea3..0994af3021 100644
--- a/src/net.cpp
+++ b/src/net.cpp
@@ -2076,6 +2076,7 @@ bool CConnman::BindListenPort(const CService &addrBind, std::string& strError, b
 
     // Set to non-blocking, incoming connections will also inherit this
     if (!SetSocketNonBlocking(hListenSocket, true)) {
+        CloseSocket(hListenSocket);
         strError = strprintf("BindListenPort: Setting listening socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError()));
         LogPrintf("%s\n", strError);
         return false;
diff --git a/src/netbase.cpp b/src/netbase.cpp
index 1f668a5d4c..8952468ecd 100644
--- a/src/netbase.cpp
+++ b/src/netbase.cpp
@@ -203,7 +203,7 @@ enum class IntrRecvError {
  *
  * @note This function requires that hSocket is in non-blocking mode.
  */
-static IntrRecvError InterruptibleRecv(char* data, size_t len, int timeout, SOCKET& hSocket)
+static IntrRecvError InterruptibleRecv(char* data, size_t len, int timeout, const SOCKET& hSocket)
 {
     int64_t curTime = GetTimeMillis();
     int64_t endTime = curTime + timeout;
@@ -424,8 +424,10 @@ bool static ConnectSocketDirectly(const CService &addrConnect, SOCKET& hSocketRe
     SetSocketNoDelay(hSocket);
 
     // Set to non-blocking
-    if (!SetSocketNonBlocking(hSocket, true))
+    if (!SetSocketNonBlocking(hSocket, true)) {
+        CloseSocket(hSocket);
         return error("ConnectSocketDirectly: Setting socket to non-blocking failed, error %s\n", NetworkErrorString(WSAGetLastError()));
+    }
 
     if (connect(hSocket, (struct sockaddr*)&sockaddr, len) == SOCKET_ERROR)
     {
@@ -682,7 +684,7 @@ bool CloseSocket(SOCKET& hSocket)
     return ret != SOCKET_ERROR;
 }
 
-bool SetSocketNonBlocking(SOCKET& hSocket, bool fNonBlocking)
+bool SetSocketNonBlocking(const SOCKET& hSocket, bool fNonBlocking)
 {
     if (fNonBlocking) {
 #ifdef WIN32
@@ -692,7 +694,6 @@ bool SetSocketNonBlocking(SOCKET& hSocket, bool fNonBlocking)
         int fFlags = fcntl(hSocket, F_GETFL, 0);
         if (fcntl(hSocket, F_SETFL, fFlags | O_NONBLOCK) == SOCKET_ERROR) {
 #endif
-            CloseSocket(hSocket);
             return false;
         }
     } else {
@@ -703,7 +704,6 @@ bool SetSocketNonBlocking(SOCKET& hSocket, bool fNonBlocking)
         int fFlags = fcntl(hSocket, F_GETFL, 0);
         if (fcntl(hSocket, F_SETFL, fFlags & ~O_NONBLOCK) == SOCKET_ERROR) {
 #endif
-            CloseSocket(hSocket);
             return false;
         }
     }
@@ -711,7 +711,7 @@ bool SetSocketNonBlocking(SOCKET& hSocket, bool fNonBlocking)
     return true;
 }
 
-bool SetSocketNoDelay(SOCKET& hSocket)
+bool SetSocketNoDelay(const SOCKET& hSocket)
 {
     int set = 1;
     int rc = setsockopt(hSocket, IPPROTO_TCP, TCP_NODELAY, (const char*)&set, sizeof(int));
diff --git a/src/netbase.h b/src/netbase.h
index fd4b34c8f1..941da31f9c 100644
--- a/src/netbase.h
+++ b/src/netbase.h
@@ -57,9 +57,9 @@ std::string NetworkErrorString(int err);
 /** Close socket and set hSocket to INVALID_SOCKET */
 bool CloseSocket(SOCKET& hSocket);
 /** Disable or enable blocking-mode for a socket */
-bool SetSocketNonBlocking(SOCKET& hSocket, bool fNonBlocking);
+bool SetSocketNonBlocking(const SOCKET& hSocket, bool fNonBlocking);
 /** Set the TCP_NODELAY flag on a socket */
-bool SetSocketNoDelay(SOCKET& hSocket);
+bool SetSocketNoDelay(const SOCKET& hSocket);
 /**
  * Convert milliseconds to a struct timeval for e.g. select.
  */