Skip to content

Commit 6ba0aab

Browse files
vasildvasild
committed
style: modernize the style of SockMan::BindListenPort()
It was copied verbatim from `CConnman::BindListenPort()` in the previous commit. Modernize its variables and style and log the error messages from the caller. Also categorize the informative messages to the "net" category because they are quite specific to the networking layer.
1 parent 0a22cda commit 6ba0aab

File tree

4 files changed

+57
-45
lines changed

4 files changed

+57
-45
lines changed

‎src/common/sockman.cpp‎

Lines changed: 44 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -9,68 +9,77 @@
99
#include <netbase.h>
1010
#include <util/sock.h>
1111

12-
bool SockMan::BindListenPort(const CService& addrBind, bilingual_str& strError)
12+
bool SockMan::BindAndStartListening(const CService& to, bilingual_str& err_msg)
1313
{
14-
int nOne = 1;
15-
1614
// Create socket for listening for incoming connections
17-
struct sockaddr_storage sockaddr;
18-
socklen_t len = sizeof(sockaddr);
19-
if (!addrBind.GetSockAddr((struct sockaddr*)&sockaddr, &len))
20-
{
21-
strError = Untranslated(strprintf("Bind address family for %s not supported", addrBind.ToStringAddrPort()));
22-
LogPrintLevel(BCLog::NET, BCLog::Level::Error, "%s\n", strError.original);
15+
sockaddr_storage storage;
16+
socklen_t len{sizeof(storage)};
17+
if (!to.GetSockAddr(reinterpret_cast<sockaddr*>(&storage), &len)) {
18+
err_msg = Untranslated(strprintf("Bind address family for %s not supported", to.ToStringAddrPort()));
2319
return false;
2420
}
2521

26-
std::unique_ptr<Sock> sock = CreateSock(addrBind.GetSAFamily(), SOCK_STREAM, IPPROTO_TCP);
22+
std::unique_ptr<Sock> sock{CreateSock(to.GetSAFamily(), SOCK_STREAM, IPPROTO_TCP)};
2723
if (!sock) {
28-
strError = Untranslated(strprintf("Couldn't open socket for incoming connections (socket returned error %s)", NetworkErrorString(WSAGetLastError())));
29-
LogPrintLevel(BCLog::NET, BCLog::Level::Error, "%s\n", strError.original);
24+
err_msg = Untranslated(strprintf("Cannot create %s listen socket: %s",
25+
to.ToStringAddrPort(),
26+
NetworkErrorString(WSAGetLastError())));
3027
return false;
3128
}
3229

30+
int one{1};
31+
3332
// Allow binding if the port is still in TIME_WAIT state after
3433
// the program was closed and restarted.
35-
if (sock->SetSockOpt(SOL_SOCKET, SO_REUSEADDR, &nOne, sizeof(int)) == SOCKET_ERROR) {
36-
strError = Untranslated(strprintf("Error setting SO_REUSEADDR on socket: %s, continuing anyway", NetworkErrorString(WSAGetLastError())));
37-
LogPrintf("%s\n", strError.original);
34+
if (sock->SetSockOpt(SOL_SOCKET, SO_REUSEADDR, &one, sizeof(one)) == SOCKET_ERROR) {
35+
LogPrintLevel(BCLog::NET,
36+
BCLog::Level::Info,
37+
"Cannot set SO_REUSEADDR on %s listen socket: %s, continuing anyway\n",
38+
to.ToStringAddrPort(),
39+
NetworkErrorString(WSAGetLastError()));
3840
}
3941

4042
// some systems don't have IPV6_V6ONLY but are always v6only; others do have the option
4143
// and enable it by default or not. Try to enable it, if possible.
42-
if (addrBind.IsIPv6()) {
44+
if (to.IsIPv6()) {
4345
#ifdef IPV6_V6ONLY
44-
if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_V6ONLY, &nOne, sizeof(int)) == SOCKET_ERROR) {
45-
strError = Untranslated(strprintf("Error setting IPV6_V6ONLY on socket: %s, continuing anyway", NetworkErrorString(WSAGetLastError())));
46-
LogPrintf("%s\n", strError.original);
46+
if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_V6ONLY, &one, sizeof(one)) == SOCKET_ERROR) {
47+
LogPrintLevel(BCLog::NET,
48+
BCLog::Level::Info,
49+
"Cannot set IPV6_V6ONLY on %s listen socket: %s, continuing anyway\n",
50+
to.ToStringAddrPort(),
51+
NetworkErrorString(WSAGetLastError()));
4752
}
4853
#endif
4954
#ifdef WIN32
50-
int nProtLevel = PROTECTION_LEVEL_UNRESTRICTED;
51-
if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_PROTECTION_LEVEL, &nProtLevel, sizeof(int)) == SOCKET_ERROR) {
52-
strError = Untranslated(strprintf("Error setting IPV6_PROTECTION_LEVEL on socket: %s, continuing anyway", NetworkErrorString(WSAGetLastError())));
53-
LogPrintf("%s\n", strError.original);
55+
int prot_level{PROTECTION_LEVEL_UNRESTRICTED};
56+
if (sock->SetSockOpt(IPPROTO_IPV6, IPV6_PROTECTION_LEVEL, &prot_level, sizeof(prot_level)) == SOCKET_ERROR) {
57+
LogPrintLevel(BCLog::NET,
58+
BCLog::Level::Info,
59+
"Cannot set IPV6_PROTECTION_LEVEL on %s listen socket: %s, continuing anyway\n",
60+
to.ToStringAddrPort(),
61+
NetworkErrorString(WSAGetLastError()));
5462
}
5563
#endif
5664
}
5765

58-
if (sock->Bind(reinterpret_cast<struct sockaddr*>(&sockaddr), len) == SOCKET_ERROR) {
59-
int nErr = WSAGetLastError();
60-
if (nErr == WSAEADDRINUSE)
61-
strError = strprintf(_("Unable to bind to %s on this computer. %s is probably already running."), addrBind.ToStringAddrPort(), CLIENT_NAME);
62-
else
63-
strError = strprintf(_("Unable to bind to %s on this computer (bind returned error %s)"), addrBind.ToStringAddrPort(), NetworkErrorString(nErr));
64-
LogPrintLevel(BCLog::NET, BCLog::Level::Error, "%s\n", strError.original);
66+
if (sock->Bind(reinterpret_cast<sockaddr*>(&storage), len) == SOCKET_ERROR) {
67+
const int err{WSAGetLastError()};
68+
if (err == WSAEADDRINUSE) {
69+
err_msg = strprintf(_("Unable to bind to %s on this computer. %s is probably already running."),
70+
to.ToStringAddrPort(),
71+
CLIENT_NAME);
72+
} else {
73+
err_msg = strprintf(_("Unable to bind to %s on this computer (bind returned error %s)"),
74+
to.ToStringAddrPort(),
75+
NetworkErrorString(err));
76+
}
6577
return false;
6678
}
67-
LogPrintf("Bound to %s\n", addrBind.ToStringAddrPort());
6879

6980
// Listen for incoming connections
70-
if (sock->Listen(SOMAXCONN) == SOCKET_ERROR)
71-
{
72-
strError = strprintf(_("Listening for incoming connections failed (listen returned error %s)"), NetworkErrorString(WSAGetLastError()));
73-
LogPrintLevel(BCLog::NET, BCLog::Level::Error, "%s\n", strError.original);
81+
if (sock->Listen(SOMAXCONN) == SOCKET_ERROR) {
82+
err_msg = strprintf(_("Cannot listen on %s: %s"), to.ToStringAddrPort(), NetworkErrorString(WSAGetLastError()));
7483
return false;
7584
}
7685

‎src/common/sockman.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ class SockMan
2323
public:
2424
/**
2525
* Bind to a new address:port, start listening and add the listen socket to `m_listen`.
26-
* @param[in] addrBind Where to bind.
27-
* @param[out] strError Error string if an error occurs.
26+
* @param[in] to Where to bind.
27+
* @param[out] err_msg Error string if an error occurs.
2828
* @retval true Success.
29-
* @retval false Failure, `strError` will be set.
29+
* @retval false Failure, `err_msg` will be set.
3030
*/
31-
bool BindListenPort(const CService& addrBind, bilingual_str& strError);
31+
bool BindAndStartListening(const CService& to, bilingual_str& err_msg);
3232

3333
/**
3434
* Stop listening by closing all listening sockets.

‎src/net.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3200,13 +3200,16 @@ bool CConnman::Bind(const CService& addr_, unsigned int flags, NetPermissionFlag
32003200
const CService addr{MaybeFlipIPv6toCJDNS(addr_)};
32013201

32023202
bilingual_str strError;
3203-
if (!BindListenPort(addr, strError)) {
3203+
if (!BindAndStartListening(addr, strError)) {
3204+
LogError("%s", strError.original);
32043205
if ((flags & BF_REPORT_ERROR) && m_client_interface) {
32053206
m_client_interface->ThreadSafeMessageBox(strError, "", CClientUIInterface::MSG_ERROR);
32063207
}
32073208
return false;
32083209
}
32093210

3211+
LogPrintLevel(BCLog::NET, BCLog::Level::Info, "Bound to and listening on %s", addr.ToStringAddrPort());
3212+
32103213
m_listen_permissions.emplace(addr, permissions);
32113214

32123215
if (addr.IsRoutable() && fDiscover && !(flags & BF_DONT_ADVERTISE) && !NetPermissions::HasFlag(permissions, NetPermissionFlags::NoBan)) {

‎test/functional/feature_port.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,23 +29,23 @@ def run_test(self):
2929
port2 = p2p_port(self.num_nodes + 5)
3030

3131
self.log.info("When starting with -port, bitcoind binds to it and uses port + 1 for an onion bind")
32-
with node.assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port1}', f'Bound to 127.0.0.1:{port1 + 1}']):
32+
with node.assert_debug_log(expected_msgs=[f'Bound to and listening on 0.0.0.0:{port1}', f'Bound to and listening on 127.0.0.1:{port1 + 1}']):
3333
self.restart_node(0, extra_args=["-listen", f"-port={port1}"])
3434

3535
self.log.info("When specifying -port multiple times, only the last one is taken")
36-
with node.assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port2}', f'Bound to 127.0.0.1:{port2 + 1}'], unexpected_msgs=[f'Bound to 0.0.0.0:{port1}']):
36+
with node.assert_debug_log(expected_msgs=[f'Bound to and listening on 0.0.0.0:{port2}', f'Bound to and listening on 127.0.0.1:{port2 + 1}'], unexpected_msgs=[f'Bound to and listening on 0.0.0.0:{port1}']):
3737
self.restart_node(0, extra_args=["-listen", f"-port={port1}", f"-port={port2}"])
3838

3939
self.log.info("When specifying ports with both -port and -bind, the one from -port is ignored")
40-
with node.assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port2}'], unexpected_msgs=[f'Bound to 0.0.0.0:{port1}']):
40+
with node.assert_debug_log(expected_msgs=[f'Bound to and listening on 0.0.0.0:{port2}'], unexpected_msgs=[f'Bound to and listening on 0.0.0.0:{port1}']):
4141
self.restart_node(0, extra_args=["-listen", f"-port={port1}", f"-bind=0.0.0.0:{port2}"])
4242

4343
self.log.info("When -bind specifies no port, the values from -port and -bind are combined")
44-
with self.nodes[0].assert_debug_log(expected_msgs=[f'Bound to 0.0.0.0:{port1}']):
44+
with self.nodes[0].assert_debug_log(expected_msgs=[f'Bound to and listening on 0.0.0.0:{port1}']):
4545
self.restart_node(0, extra_args=["-listen", f"-port={port1}", "-bind=0.0.0.0"])
4646

4747
self.log.info("When an onion bind specifies no port, the value from -port, incremented by 1, is taken")
48-
with self.nodes[0].assert_debug_log(expected_msgs=[f'Bound to 127.0.0.1:{port1 + 1}']):
48+
with self.nodes[0].assert_debug_log(expected_msgs=[f'Bound to and listening on 127.0.0.1:{port1 + 1}']):
4949
self.restart_node(0, extra_args=["-listen", f"-port={port1}", "-bind=127.0.0.1=onion"])
5050

5151
self.log.info("Invalid values for -port raise errors")

0 commit comments

Comments
 (0)