net: change CreateNodeFromAcceptedSocket() to take Sock

Change `CConnman::CreateNodeFromAcceptedSocket()` to take a `Sock`
argument instead of `SOCKET`.

This makes the method mockable and also a little bit shorter as some
`CloseSocket()` calls are removed (the socket will be closed
automatically by the `Sock` destructor on early return).
This commit is contained in:
Vasil Dimov 2021-04-13 12:14:57 +02:00
parent 9e3cbfca7c
commit 6bf6e9fd9d
No known key found for this signature in database
GPG Key ID: 54DF06F64B55CBBF
2 changed files with 8 additions and 13 deletions

View File

@ -1120,10 +1120,10 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
NetPermissionFlags permissionFlags = NetPermissionFlags::None; NetPermissionFlags permissionFlags = NetPermissionFlags::None;
hListenSocket.AddSocketPermissionFlags(permissionFlags); hListenSocket.AddSocketPermissionFlags(permissionFlags);
CreateNodeFromAcceptedSocket(sock->Release(), permissionFlags, addr_bind, addr); CreateNodeFromAcceptedSocket(std::move(sock), permissionFlags, addr_bind, addr);
} }
void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket, void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
NetPermissionFlags permissionFlags, NetPermissionFlags permissionFlags,
const CAddress& addr_bind, const CAddress& addr_bind,
const CAddress& addr) const CAddress& addr)
@ -1149,27 +1149,24 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket,
if (!fNetworkActive) { if (!fNetworkActive) {
LogPrint(BCLog::NET, "connection from %s dropped: not accepting new connections\n", addr.ToString()); LogPrint(BCLog::NET, "connection from %s dropped: not accepting new connections\n", addr.ToString());
CloseSocket(hSocket);
return; return;
} }
if (!IsSelectableSocket(hSocket)) if (!IsSelectableSocket(sock->Get()))
{ {
LogPrintf("connection from %s dropped: non-selectable socket\n", addr.ToString()); LogPrintf("connection from %s dropped: non-selectable socket\n", addr.ToString());
CloseSocket(hSocket);
return; return;
} }
// According to the internet TCP_NODELAY is not carried into accepted sockets // According to the internet TCP_NODELAY is not carried into accepted sockets
// on all platforms. Set it again here just to be sure. // on all platforms. Set it again here just to be sure.
SetSocketNoDelay(hSocket); SetSocketNoDelay(sock->Get());
// Don't accept connections from banned peers. // Don't accept connections from banned peers.
bool banned = m_banman && m_banman->IsBanned(addr); bool banned = m_banman && m_banman->IsBanned(addr);
if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::NoBan) && banned) if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::NoBan) && banned)
{ {
LogPrint(BCLog::NET, "connection from %s dropped (banned)\n", addr.ToString()); LogPrint(BCLog::NET, "connection from %s dropped (banned)\n", addr.ToString());
CloseSocket(hSocket);
return; return;
} }
@ -1178,7 +1175,6 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket,
if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::NoBan) && nInbound + 1 >= nMaxInbound && discouraged) if (!NetPermissions::HasFlag(permissionFlags, NetPermissionFlags::NoBan) && nInbound + 1 >= nMaxInbound && discouraged)
{ {
LogPrint(BCLog::NET, "connection from %s dropped (discouraged)\n", addr.ToString()); LogPrint(BCLog::NET, "connection from %s dropped (discouraged)\n", addr.ToString());
CloseSocket(hSocket);
return; return;
} }
@ -1187,7 +1183,6 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket,
if (!AttemptToEvictConnection()) { if (!AttemptToEvictConnection()) {
// No connection to evict, disconnect the new connection // No connection to evict, disconnect the new connection
LogPrint(BCLog::NET, "failed to find an eviction candidate - connection dropped (full)\n"); LogPrint(BCLog::NET, "failed to find an eviction candidate - connection dropped (full)\n");
CloseSocket(hSocket);
return; return;
} }
} }
@ -1201,7 +1196,7 @@ void CConnman::CreateNodeFromAcceptedSocket(SOCKET hSocket,
} }
const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end(); const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end();
CNode* pnode = new CNode(id, nodeServices, hSocket, addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", ConnectionType::INBOUND, inbound_onion); CNode* pnode = new CNode(id, nodeServices, sock->Release(), addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", ConnectionType::INBOUND, inbound_onion);
pnode->AddRef(); pnode->AddRef();
pnode->m_permissionFlags = permissionFlags; pnode->m_permissionFlags = permissionFlags;
pnode->m_prefer_evict = discouraged; pnode->m_prefer_evict = discouraged;
@ -2329,7 +2324,7 @@ void CConnman::ThreadI2PAcceptIncoming()
continue; continue;
} }
CreateNodeFromAcceptedSocket(conn.sock->Release(), NetPermissionFlags::None, CreateNodeFromAcceptedSocket(std::move(conn.sock), NetPermissionFlags::None,
CAddress{conn.me, NODE_NONE}, CAddress{conn.peer, NODE_NONE}); CAddress{conn.me, NODE_NONE}, CAddress{conn.peer, NODE_NONE});
} }
} }

View File

@ -974,12 +974,12 @@ private:
/** /**
* Create a `CNode` object from a socket that has just been accepted and add the node to * Create a `CNode` object from a socket that has just been accepted and add the node to
* the `m_nodes` member. * the `m_nodes` member.
* @param[in] hSocket Connected socket to communicate with the peer. * @param[in] sock Connected socket to communicate with the peer.
* @param[in] permissionFlags The peer's permissions. * @param[in] permissionFlags The peer's permissions.
* @param[in] addr_bind The address and port at our side of the connection. * @param[in] addr_bind The address and port at our side of the connection.
* @param[in] addr The address and port at the peer's side of the connection. * @param[in] addr The address and port at the peer's side of the connection.
*/ */
void CreateNodeFromAcceptedSocket(SOCKET hSocket, void CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
NetPermissionFlags permissionFlags, NetPermissionFlags permissionFlags,
const CAddress& addr_bind, const CAddress& addr_bind,
const CAddress& addr); const CAddress& addr);