Skip to content

Commit 18cd088

Browse files
author
MarcoFalke
committed
Merge #21328: net, refactor: pass uint16 CService::port as uint16
52dd40a test: add missing netaddress include headers (Jon Atack) 6f09c0f util: add missing braces and apply clang format to SplitHostPort() (Jon Atack) 2875a76 util: add ParseUInt16(), use it in SplitHostPort() (Jon Atack) 6423c81 p2p, refactor: pass and use uint16_t CService::port as uint16_t (Jon Atack) Pull request description: As noticed during review today in bitcoin/bitcoin#20685 (comment) of the upcoming I2P network support, `CService::port` is `uint16_t` but is passed around the codebase and into the ctors as `int`, which causes uneeded conversions and casts. We can avoid these (including in the incoming I2P code without further changes to it) by using ports with the correct type. The remaining conversions are pushed out to the user input boundaries where they can be range-checked and raise with user feedback in the next patch. ACKs for top commit: practicalswift: cr ACK 52dd40a: patch looks correct MarcoFalke: cr ACK 52dd40a vasild: ACK 52dd40a Tree-SHA512: 203c1cab3189a206c55ecada77b9548b810281cdc533252b8e3330ae0606b467731c75f730ce9deb07cbaab66facf97e1ffd2051084ff9077cba6750366b0432
2 parents 7f3fd34 + 52dd40a commit 18cd088

18 files changed

+82
-56
lines changed

src/bitcoin-cli.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -618,9 +618,9 @@ static UniValue CallRPC(BaseRequestHandler* rh, const std::string& strMethod, co
618618
// 1. -rpcport
619619
// 2. port in -rpcconnect (ie following : in ipv4 or ]: in ipv6)
620620
// 3. default port for chain
621-
int port = BaseParams().RPCPort();
621+
uint16_t port{BaseParams().RPCPort()};
622622
SplitHostPort(gArgs.GetArg("-rpcconnect", DEFAULT_RPCCONNECT), port, host);
623-
port = gArgs.GetArg("-rpcport", port);
623+
port = static_cast<uint16_t>(gArgs.GetArg("-rpcport", port));
624624

625625
// Obtain event base
626626
raii_event_base base = obtain_event_base();

src/chainparams.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ class CChainParams
8484

8585
const Consensus::Params& GetConsensus() const { return consensus; }
8686
const CMessageHeader::MessageStartChars& MessageStart() const { return pchMessageStart; }
87-
int GetDefaultPort() const { return nDefaultPort; }
87+
uint16_t GetDefaultPort() const { return nDefaultPort; }
8888

8989
const CBlock& GenesisBlock() const { return genesis; }
9090
/** Default value for -checkmempool and -checkblockindex argument */
@@ -121,7 +121,7 @@ class CChainParams
121121

122122
Consensus::Params consensus;
123123
CMessageHeader::MessageStartChars pchMessageStart;
124-
int nDefaultPort;
124+
uint16_t nDefaultPort;
125125
uint64_t nPruneAfterHeight;
126126
uint64_t m_assumed_blockchain_size;
127127
uint64_t m_assumed_chain_state_size;

src/httpserver.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -290,8 +290,8 @@ static bool ThreadHTTP(struct event_base* base)
290290
/** Bind HTTP server to specified addresses */
291291
static bool HTTPBindAddresses(struct evhttp* http)
292292
{
293-
int http_port = gArgs.GetArg("-rpcport", BaseParams().RPCPort());
294-
std::vector<std::pair<std::string, uint16_t> > endpoints;
293+
uint16_t http_port{static_cast<uint16_t>(gArgs.GetArg("-rpcport", BaseParams().RPCPort()))};
294+
std::vector<std::pair<std::string, uint16_t>> endpoints;
295295

296296
// Determine what addresses to bind to
297297
if (!(gArgs.IsArgSet("-rpcallowip") && gArgs.IsArgSet("-rpcbind"))) { // Default to loopback if not allowing external IPs
@@ -305,7 +305,7 @@ static bool HTTPBindAddresses(struct evhttp* http)
305305
}
306306
} else if (gArgs.IsArgSet("-rpcbind")) { // Specific bind address
307307
for (const std::string& strRPCBind : gArgs.GetArgs("-rpcbind")) {
308-
int port = http_port;
308+
uint16_t port{http_port};
309309
std::string host;
310310
SplitHostPort(strRPCBind, port, host);
311311
endpoints.push_back(std::make_pair(host, port));

src/net.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ void CConnman::AddAddrFetch(const std::string& strDest)
113113

114114
uint16_t GetListenPort()
115115
{
116-
return (uint16_t)(gArgs.GetArg("-port", Params().GetDefaultPort()));
116+
return static_cast<uint16_t>(gArgs.GetArg("-port", Params().GetDefaultPort()));
117117
}
118118

119119
// find 'best' local address for a particular peer
@@ -394,7 +394,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
394394
pszDest ? 0.0 : (double)(GetAdjustedTime() - addrConnect.nTime)/3600.0);
395395

396396
// Resolve
397-
const int default_port = Params().GetDefaultPort();
397+
const uint16_t default_port{Params().GetDefaultPort()};
398398
if (pszDest) {
399399
std::vector<CService> resolved;
400400
if (Lookup(pszDest, resolved, default_port, fNameLookup && !HaveNameProxy(), 256) && !resolved.empty()) {
@@ -462,7 +462,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
462462
return nullptr;
463463
}
464464
std::string host;
465-
int port = default_port;
465+
uint16_t port{default_port};
466466
SplitHostPort(std::string(pszDest), port, host);
467467
bool proxyConnectionFailed;
468468
connected = ConnectThroughProxy(proxy, host, port, *sock, nConnectTimeout,

src/net.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,7 @@ extern std::string strSubVersion;
229229

230230
struct LocalServiceInfo {
231231
int nScore;
232-
int nPort;
232+
uint16_t nPort;
233233
};
234234

235235
extern RecursiveMutex cs_mapLocalHost;

src/netbase.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -194,12 +194,12 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSL
194194
return true;
195195
}
196196

197-
bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function)
197+
bool Lookup(const std::string& name, std::vector<CService>& vAddr, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function)
198198
{
199199
if (name.empty() || !ValidAsCString(name)) {
200200
return false;
201201
}
202-
int port = portDefault;
202+
uint16_t port{portDefault};
203203
std::string hostname;
204204
SplitHostPort(name, port, hostname);
205205

@@ -213,7 +213,7 @@ bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefau
213213
return true;
214214
}
215215

216-
bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function)
216+
bool Lookup(const std::string& name, CService& addr, uint16_t portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function)
217217
{
218218
if (!ValidAsCString(name)) {
219219
return false;
@@ -226,7 +226,7 @@ bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllo
226226
return true;
227227
}
228228

229-
CService LookupNumeric(const std::string& name, int portDefault, DNSLookupFn dns_lookup_function)
229+
CService LookupNumeric(const std::string& name, uint16_t portDefault, DNSLookupFn dns_lookup_function)
230230
{
231231
if (!ValidAsCString(name)) {
232232
return {};
@@ -363,7 +363,7 @@ static std::string Socks5ErrorString(uint8_t err)
363363
}
364364
}
365365

366-
bool Socks5(const std::string& strDest, int port, const ProxyCredentials* auth, const Sock& sock)
366+
bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* auth, const Sock& sock)
367367
{
368368
IntrRecvError recvr;
369369
LogPrint(BCLog::NET, "SOCKS5 connecting %s\n", strDest);
@@ -665,7 +665,7 @@ bool IsProxy(const CNetAddr &addr) {
665665
return false;
666666
}
667667

668-
bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, int port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed)
668+
bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, uint16_t port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed)
669669
{
670670
// first connect to proxy server
671671
if (!ConnectSocketDirectly(proxy.proxy, sock.Get(), nTimeout, true)) {
@@ -677,11 +677,11 @@ bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, int
677677
ProxyCredentials random_auth;
678678
static std::atomic_int counter(0);
679679
random_auth.username = random_auth.password = strprintf("%i", counter++);
680-
if (!Socks5(strDest, (uint16_t)port, &random_auth, sock)) {
680+
if (!Socks5(strDest, port, &random_auth, sock)) {
681681
return false;
682682
}
683683
} else {
684-
if (!Socks5(strDest, (uint16_t)port, 0, sock)) {
684+
if (!Socks5(strDest, port, 0, sock)) {
685685
return false;
686686
}
687687
}

src/netbase.h

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -111,15 +111,15 @@ extern DNSLookupFn g_dns_lookup;
111111
* @returns Whether or not the specified host string successfully resolved to
112112
* any resulting network addresses.
113113
*
114-
* @see Lookup(const std::string&, std::vector<CService>&, int, bool, unsigned int, DNSLookupFn)
114+
* @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn)
115115
* for additional parameter descriptions.
116116
*/
117117
bool LookupHost(const std::string& name, std::vector<CNetAddr>& vIP, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup);
118118

119119
/**
120120
* Resolve a host string to its first corresponding network address.
121121
*
122-
* @see LookupHost(const std::string&, std::vector<CNetAddr>&, unsigned int, bool, DNSLookupFn)
122+
* @see LookupHost(const std::string&, std::vector<CNetAddr>&, uint16_t, bool, DNSLookupFn)
123123
* for additional parameter descriptions.
124124
*/
125125
bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup);
@@ -129,7 +129,7 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSL
129129
*
130130
* @param name The string representing a service. Could be a name or a
131131
* numerical IP address (IPv6 addresses should be in their
132-
* disambiguated bracketed form), optionally followed by a port
132+
* disambiguated bracketed form), optionally followed by a uint16_t port
133133
* number. (e.g. example.com:8333 or
134134
* [2001:db8:85a3:8d3:1319:8a2e:370:7348]:420)
135135
* @param[out] vAddr The resulting services to which the specified service string
@@ -144,26 +144,26 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSL
144144
* @returns Whether or not the service string successfully resolved to any
145145
* resulting services.
146146
*/
147-
bool Lookup(const std::string& name, std::vector<CService>& vAddr, int portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function = g_dns_lookup);
147+
bool Lookup(const std::string& name, std::vector<CService>& vAddr, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function = g_dns_lookup);
148148

149149
/**
150150
* Resolve a service string to its first corresponding service.
151151
*
152-
* @see Lookup(const std::string&, std::vector<CService>&, int, bool, unsigned int, DNSLookupFn)
152+
* @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn)
153153
* for additional parameter descriptions.
154154
*/
155-
bool Lookup(const std::string& name, CService& addr, int portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup);
155+
bool Lookup(const std::string& name, CService& addr, uint16_t portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup);
156156

157157
/**
158158
* Resolve a service string with a numeric IP to its first corresponding
159159
* service.
160160
*
161161
* @returns The resulting CService if the resolution was successful, [::]:0 otherwise.
162162
*
163-
* @see Lookup(const std::string&, std::vector<CService>&, int, bool, unsigned int, DNSLookupFn)
163+
* @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn)
164164
* for additional parameter descriptions.
165165
*/
166-
CService LookupNumeric(const std::string& name, int portDefault = 0, DNSLookupFn dns_lookup_function = g_dns_lookup);
166+
CService LookupNumeric(const std::string& name, uint16_t portDefault = 0, DNSLookupFn dns_lookup_function = g_dns_lookup);
167167

168168
/**
169169
* Parse and resolve a specified subnet string into the appropriate internal
@@ -219,7 +219,7 @@ bool ConnectSocketDirectly(const CService &addrConnect, const SOCKET& hSocket, i
219219
*
220220
* @returns Whether or not the operation succeeded.
221221
*/
222-
bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, int port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed);
222+
bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, uint16_t port, const Sock& sock, int nTimeout, bool& outProxyConnectionFailed);
223223

224224
/** Disable or enable blocking-mode for a socket */
225225
bool SetSocketNonBlocking(const SOCKET& hSocket, bool fNonBlocking);
@@ -245,6 +245,6 @@ void InterruptSocks5(bool interrupt);
245245
* @see <a href="https://www.ietf.org/rfc/rfc1928.txt">RFC1928: SOCKS Protocol
246246
* Version 5</a>
247247
*/
248-
bool Socks5(const std::string& strDest, int port, const ProxyCredentials* auth, const Sock& socket);
248+
bool Socks5(const std::string& strDest, uint16_t port, const ProxyCredentials* auth, const Sock& socket);
249249

250250
#endif // BITCOIN_NETBASE_H

src/qt/optionsdialog.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ private Q_SLOTS:
6767
void updateDefaultProxyNets();
6868

6969
Q_SIGNALS:
70-
void proxyIpChecks(QValidatedLineEdit *pUiProxyIp, int nProxyPort);
70+
void proxyIpChecks(QValidatedLineEdit *pUiProxyIp, uint16_t nProxyPort);
7171

7272
private:
7373
Ui::OptionsDialog *ui;

src/rpc/net.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -914,7 +914,7 @@ static RPCHelpMan addpeeraddress()
914914
UniValue obj(UniValue::VOBJ);
915915

916916
std::string addr_string = request.params[0].get_str();
917-
uint16_t port = request.params[1].get_int();
917+
uint16_t port{static_cast<uint16_t>(request.params[1].get_int())};
918918

919919
CNetAddr net_addr;
920920
if (!LookupHost(addr_string, net_addr, false)) {

src/test/addrman_tests.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@ static CNetAddr ResolveIP(const std::string& ip)
100100
return addr;
101101
}
102102

103-
static CService ResolveService(const std::string& ip, const int port = 0)
103+
static CService ResolveService(const std::string& ip, uint16_t port = 0)
104104
{
105105
CService serv;
106106
BOOST_CHECK_MESSAGE(Lookup(ip, serv, port, false), strprintf("failed to resolve: %s:%i", ip, port));

0 commit comments

Comments
 (0)