net: move-only: improve encapsulation of SockMan

`SockMan` members `AcceptConnection()` `NewSockAccepted()` `GetNewNodeId()` `m_i2p_sam_session` `m_listen private` are now used only by `SockMan`, thus make them private.
vasild · Sep 25, 2024 · da33f74 · da33f74
1 parent b8f0505
commit da33f74
Show file tree

Hide file tree

Showing 2 changed files with 94 additions and 94 deletions.
diff --git a/src/sockman.cpp b/src/sockman.cpp
@@ -232,65 +232,6 @@ SockMan::ConnectAndMakeNodeId(const std::variant<CService, StringHostIntPort>& t
     return node_id;
 }
 
-std::unique_ptr<Sock> SockMan::AcceptConnection(const Sock& listen_sock, CService& addr)
-{
-    sockaddr_storage storage;
-    socklen_t len{sizeof(storage)};
-
-    auto sock{listen_sock.Accept(reinterpret_cast<sockaddr*>(&storage), &len)};
-
-    if (!sock) {
-        const int err{WSAGetLastError()};
-        if (err != WSAEWOULDBLOCK) {
-            LogPrintLevel(BCLog::NET,
-                          BCLog::Level::Error,
-                          "Cannot accept new connection: %s\n",
-                          NetworkErrorString(err));
-        }
-        return {};
-    }
-
-    if (!addr.SetSockAddr(reinterpret_cast<sockaddr*>(&storage))) {
-        LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "Unknown socket family\n");
-    }
-
-    return sock;
-}
-
-void SockMan::NewSockAccepted(std::unique_ptr<Sock>&& sock, const CService& me, const CService& them)
-{
-    AssertLockNotHeld(m_connected_mutex);
-
-    if (!sock->IsSelectable()) {
-        LogPrintf("connection from %s dropped: non-selectable socket\n", them.ToStringAddrPort());
-        return;
-    }
-
-    // According to the internet TCP_NODELAY is not carried into accepted sockets
-    // on all platforms.  Set it again here just to be sure.
-    const int on{1};
-    if (sock->SetSockOpt(IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)) == SOCKET_ERROR) {
-        LogDebug(BCLog::NET, "connection from %s: unable to set TCP_NODELAY, continuing anyway\n",
-                 them.ToStringAddrPort());
-    }
-
-    const NodeId node_id{GetNewNodeId()};
-
-    {
-        LOCK(m_connected_mutex);
-        m_connected.emplace(node_id, std::make_shared<SyncSock>(std::move(*sock)));
-    }
-
-    if (!EventNewConnectionAccepted(node_id, me, them)) {
-        CloseConnection(node_id);
-    }
-}
-
-NodeId SockMan::GetNewNodeId()
-{
-    return m_next_node_id.fetch_add(1, std::memory_order_relaxed);
-}
-
 bool SockMan::CloseConnection(NodeId node_id)
 {
     LOCK(m_connected_mutex);
@@ -426,6 +367,65 @@ void SockMan::ThreadSocketHandler()
     }
 }
 
+std::unique_ptr<Sock> SockMan::AcceptConnection(const Sock& listen_sock, CService& addr)
+{
+    sockaddr_storage storage;
+    socklen_t len{sizeof(storage)};
+
+    auto sock{listen_sock.Accept(reinterpret_cast<sockaddr*>(&storage), &len)};
+
+    if (!sock) {
+        const int err{WSAGetLastError()};
+        if (err != WSAEWOULDBLOCK) {
+            LogPrintLevel(BCLog::NET,
+                          BCLog::Level::Error,
+                          "Cannot accept new connection: %s\n",
+                          NetworkErrorString(err));
+        }
+        return {};
+    }
+
+    if (!addr.SetSockAddr(reinterpret_cast<sockaddr*>(&storage))) {
+        LogPrintLevel(BCLog::NET, BCLog::Level::Warning, "Unknown socket family\n");
+    }
+
+    return sock;
+}
+
+void SockMan::NewSockAccepted(std::unique_ptr<Sock>&& sock, const CService& me, const CService& them)
+{
+    AssertLockNotHeld(m_connected_mutex);
+
+    if (!sock->IsSelectable()) {
+        LogPrintf("connection from %s dropped: non-selectable socket\n", them.ToStringAddrPort());
+        return;
+    }
+
+    // According to the internet TCP_NODELAY is not carried into accepted sockets
+    // on all platforms.  Set it again here just to be sure.
+    const int on{1};
+    if (sock->SetSockOpt(IPPROTO_TCP, TCP_NODELAY, &on, sizeof(on)) == SOCKET_ERROR) {
+        LogDebug(BCLog::NET, "connection from %s: unable to set TCP_NODELAY, continuing anyway\n",
+                 them.ToStringAddrPort());
+    }
+
+    const NodeId node_id{GetNewNodeId()};
+
+    {
+        LOCK(m_connected_mutex);
+        m_connected.emplace(node_id, std::make_shared<SyncSock>(std::move(*sock)));
+    }
+
+    if (!EventNewConnectionAccepted(node_id, me, them)) {
+        CloseConnection(node_id);
+    }
+}
+
+NodeId SockMan::GetNewNodeId()
+{
+    return m_next_node_id.fetch_add(1, std::memory_order_relaxed);
+}
+
 Sock::EventsPerSock SockMan::GenerateWaitSockets()
 {
     AssertLockNotHeld(m_connected_mutex);

diff --git a/src/sockman.h b/src/sockman.h
@@ -106,29 +106,6 @@ class SockMan
                                                CService& me)
         EXCLUSIVE_LOCKS_REQUIRED(!m_connected_mutex, !m_unused_i2p_sessions_mutex);
 
-    /**
-     * Accept a connection.
-     * @param[in] listen_sock Socket on which to accept the connection.
-     * @param[out] addr Address of the peer that was accepted.
-     * @return Newly created socket for the accepted connection.
-     */
-    std::unique_ptr<Sock> AcceptConnection(const Sock& listen_sock, CService& addr);
-
-    /**
-     * After a new socket with a peer has been created, configure its flags,
-     * make a new node id and call `EventNewConnectionAccepted()`.
-     * @param[in] sock The newly created socket.
-     * @param[in] me Address at our end of the connection.
-     * @param[in] them Address of the new peer.
-     */
-    void NewSockAccepted(std::unique_ptr<Sock>&& sock, const CService& me, const CService& them)
-        EXCLUSIVE_LOCKS_REQUIRED(!m_connected_mutex);
-
-    /**
-     * Generate an id for a newly created node.
-     */
-    NodeId GetNewNodeId();
-
     /**
      * Disconnect a given peer by closing its socket and release resources occupied by it.
      * @return Whether the peer existed and its socket was closed by this call.
@@ -264,18 +241,6 @@ class SockMan
      */
     CThreadInterrupt interruptNet;
 
-    /**
-     * I2P SAM session.
-     * Used to accept incoming and make outgoing I2P connections from a persistent
-     * address.
-     */
-    std::unique_ptr<i2p::sam::Session> m_i2p_sam_session;
-
-    /**
-     * List of listening sockets.
-     */
-    std::vector<std::shared_ptr<Sock>> m_listen;
-
 private:
 
     /**
@@ -297,6 +262,29 @@ class SockMan
     void ThreadSocketHandler()
         EXCLUSIVE_LOCKS_REQUIRED(!m_connected_mutex);
 
+    /**
+     * Accept a connection.
+     * @param[in] listen_sock Socket on which to accept the connection.
+     * @param[out] addr Address of the peer that was accepted.
+     * @return Newly created socket for the accepted connection.
+     */
+    std::unique_ptr<Sock> AcceptConnection(const Sock& listen_sock, CService& addr);
+
+    /**
+     * After a new socket with a peer has been created, configure its flags,
+     * make a new node id and call `EventNewConnectionAccepted()`.
+     * @param[in] sock The newly created socket.
+     * @param[in] me Address at our end of the connection.
+     * @param[in] them Address of the new peer.
+     */
+    void NewSockAccepted(std::unique_ptr<Sock>&& sock, const CService& me, const CService& them)
+        EXCLUSIVE_LOCKS_REQUIRED(!m_connected_mutex);
+
+    /**
+     * Generate an id for a newly created node.
+     */
+    NodeId GetNewNodeId();
+
     /**
      * Generate a collection of sockets to check for IO readiness.
      * @return Sockets to check for readiness.
@@ -347,6 +335,18 @@ class SockMan
      */
     std::queue<std::unique_ptr<i2p::sam::Session>> m_unused_i2p_sessions GUARDED_BY(m_unused_i2p_sessions_mutex);
 
+    /**
+     * I2P SAM session.
+     * Used to accept incoming and make outgoing I2P connections from a persistent
+     * address.
+     */
+    std::unique_ptr<i2p::sam::Session> m_i2p_sam_session;
+
+    /**
+     * List of listening sockets.
+     */
+    std::vector<std::shared_ptr<Sock>> m_listen;
+
     /**
      * A socket with serialized send/recv operations.
      */