Skip to content

Commit 82d360b

Browse files
committed
net: change ConnectSocketDirectly() to take a Sock argument
Change `ConnectSocketDirectly()` to take a `Sock` argument instead of a bare `SOCKET`. With this, use the `Sock`'s (possibly mocked) methods `Connect()`, `Wait()` and `GetSockOpt()` instead of calling the OS functions directly.
1 parent b586110 commit 82d360b

File tree

4 files changed

+27
-40
lines changed

4 files changed

+27
-40
lines changed

src/i2p.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,7 @@ Sock Session::Hello() const
279279
throw std::runtime_error("Cannot create socket");
280280
}
281281

282-
if (!ConnectSocketDirectly(m_control_host, sock->Get(), nConnectTimeout, true)) {
282+
if (!ConnectSocketDirectly(m_control_host, *sock, nConnectTimeout, true)) {
283283
throw std::runtime_error(strprintf("Cannot connect to %s", m_control_host.ToString()));
284284
}
285285

src/net.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -448,7 +448,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
448448
if (!sock) {
449449
return nullptr;
450450
}
451-
connected = ConnectSocketDirectly(addrConnect, sock->Get(), nConnectTimeout,
451+
connected = ConnectSocketDirectly(addrConnect, *sock, nConnectTimeout,
452452
conn_type == ConnectionType::MANUAL);
453453
}
454454
if (!proxyConnectionFailed) {

src/netbase.cpp

Lines changed: 23 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -537,12 +537,12 @@ static void LogConnectFailure(bool manual_connection, const char* fmt, const Arg
537537
}
538538
}
539539

540-
bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, int nTimeout, bool manual_connection)
540+
bool ConnectSocketDirectly(const CService &addrConnect, const Sock& sock, int nTimeout, bool manual_connection)
541541
{
542542
// Create a sockaddr from the specified service.
543543
struct sockaddr_storage sockaddr;
544544
socklen_t len = sizeof(sockaddr);
545-
if (hSocket == INVALID_SOCKET) {
545+
if (sock.Get() == INVALID_SOCKET) {
546546
LogPrintf("Cannot connect to %s: invalid socket\n", addrConnect.ToString());
547547
return false;
548548
}
@@ -552,55 +552,42 @@ bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, i
552552
}
553553

554554
// Connect to the addrConnect service on the hSocket socket.
555-
if (connect(hSocket, (struct sockaddr*)&sockaddr, len) == SOCKET_ERROR)
556-
{
555+
if (sock.Connect(reinterpret_cast<struct sockaddr*>(&sockaddr), len) == SOCKET_ERROR) {
557556
int nErr = WSAGetLastError();
558557
// WSAEINVAL is here because some legacy version of winsock uses it
559558
if (nErr == WSAEINPROGRESS || nErr == WSAEWOULDBLOCK || nErr == WSAEINVAL)
560559
{
561560
// Connection didn't actually fail, but is being established
562561
// asynchronously. Thus, use async I/O api (select/poll)
563562
// synchronously to check for successful connection with a timeout.
564-
#ifdef USE_POLL
565-
struct pollfd pollfd = {};
566-
pollfd.fd = hSocket;
567-
pollfd.events = POLLIN | POLLOUT;
568-
int nRet = poll(&pollfd, 1, nTimeout);
569-
#else
570-
struct timeval timeout = MillisToTimeval(nTimeout);
571-
fd_set fdset;
572-
FD_ZERO(&fdset);
573-
FD_SET(hSocket, &fdset);
574-
int nRet = select(hSocket + 1, nullptr, &fdset, nullptr, &timeout);
575-
#endif
576-
// Upon successful completion, both select and poll return the total
577-
// number of file descriptors that have been selected. A value of 0
578-
// indicates that the call timed out and no file descriptors have
579-
// been selected.
580-
if (nRet == 0)
581-
{
582-
LogPrint(BCLog::NET, "connection to %s timeout\n", addrConnect.ToString());
563+
const Sock::Event requested = Sock::RECV | Sock::SEND;
564+
Sock::Event occurred;
565+
if (!sock.Wait(std::chrono::milliseconds{nTimeout}, requested, &occurred)) {
566+
LogPrintf("wait for connect to %s failed: %s\n",
567+
addrConnect.ToString(),
568+
NetworkErrorString(WSAGetLastError()));
583569
return false;
584-
}
585-
if (nRet == SOCKET_ERROR)
586-
{
587-
LogPrintf("select() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError()));
570+
} else if (occurred == 0) {
571+
LogPrint(BCLog::NET, "connection attempt to %s timed out\n", addrConnect.ToString());
588572
return false;
589573
}
590574

591-
// Even if the select/poll was successful, the connect might not
575+
// Even if the wait was successful, the connect might not
592576
// have been successful. The reason for this failure is hidden away
593577
// in the SO_ERROR for the socket in modern systems. We read it into
594-
// nRet here.
595-
socklen_t nRetSize = sizeof(nRet);
596-
if (getsockopt(hSocket, SOL_SOCKET, SO_ERROR, (sockopt_arg_type)&nRet, &nRetSize) == SOCKET_ERROR)
597-
{
578+
// sockerr here.
579+
int sockerr;
580+
socklen_t sockerr_len = sizeof(sockerr);
581+
if (sock.GetSockOpt(SOL_SOCKET, SO_ERROR, (sockopt_arg_type)&sockerr, &sockerr_len) ==
582+
SOCKET_ERROR) {
598583
LogPrintf("getsockopt() for %s failed: %s\n", addrConnect.ToString(), NetworkErrorString(WSAGetLastError()));
599584
return false;
600585
}
601-
if (nRet != 0)
602-
{
603-
LogConnectFailure(manual_connection, "connect() to %s failed after select(): %s", addrConnect.ToString(), NetworkErrorString(nRet));
586+
if (sockerr != 0) {
587+
LogConnectFailure(manual_connection,
588+
"connect() to %s failed after wait: %s",
589+
addrConnect.ToString(),
590+
NetworkErrorString(sockerr));
604591
return false;
605592
}
606593
}
@@ -668,7 +655,7 @@ bool IsProxy(const CNetAddr &addr) {
668655
bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, int port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed)
669656
{
670657
// first connect to proxy server
671-
if (!ConnectSocketDirectly(proxy.proxy, sock.Get(), nTimeout, true)) {
658+
if (!ConnectSocketDirectly(proxy.proxy, sock, nTimeout, true)) {
672659
outProxyConnectionFailed = true;
673660
return false;
674661
}

src/netbase.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -194,15 +194,15 @@ extern std::function<std::unique_ptr<Sock>(const CService&)> CreateSock;
194194
* Try to connect to the specified service on the specified socket.
195195
*
196196
* @param addrConnect The service to which to connect.
197-
* @param hSocket The socket on which to connect.
197+
* @param sock The socket on which to connect.
198198
* @param nTimeout Wait this many milliseconds for the connection to be
199199
* established.
200200
* @param manual_connection Whether or not the connection was manually requested
201201
* (e.g. through the addnode RPC)
202202
*
203203
* @returns Whether or not a connection was successfully made.
204204
*/
205-
bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, int nTimeout, bool manual_connection);
205+
bool ConnectSocketDirectly(const CService &addrConnect, const Sock& sock, int nTimeout, bool manual_connection);
206206

207207
/**
208208
* Connect to a specified destination service through a SOCKS5 proxy by first

0 commit comments

Comments
 (0)