Skip to content

Commit 34bcdfc

Browse files
brunoergbrunoerg
committed
p2p, refactor: return vector/optional<CService> in Lookup 
1 parent 7799eb1 commit 34bcdfc

File tree

9 files changed

+83
-100
lines changed

9 files changed

+83
-100
lines changed

‎src/bench/addrman.cpp

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,6 @@ static CNetAddr ResolveIP(const std::string& ip)
7979
return addr;
8080
}
8181

82-
static CService ResolveService(const std::string& ip, uint16_t port = 0)
83-
{
84-
CService serv;
85-
Lookup(ip, serv, port, false);
86-
return serv;
87-
}
88-
8982
/* Benchmarks */
9083

9184
static void AddrManAdd(benchmark::Bench& bench)
@@ -118,8 +111,8 @@ static void AddrManSelectFromAlmostEmpty(benchmark::Bench& bench)
118111
AddrMan addrman{EMPTY_NETGROUPMAN, /*deterministic=*/false, ADDRMAN_CONSISTENCY_CHECK_RATIO};
119112

120113
// Add one address to the new table
121-
CService addr = ResolveService("250.3.1.1", 8333);
122-
addrman.Add({CAddress(addr, NODE_NONE)}, ResolveService("250.3.1.1", 8333));
114+
CService addr = Lookup("250.3.1.1", 8333, false).value();
115+
addrman.Add({CAddress(addr, NODE_NONE)}, addr);
123116

124117
bench.run([&] {
125118
(void)addrman.Select();

‎src/init.cpp

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1355,12 +1355,12 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
13551355
// -noproxy (or -proxy=0) as well as the empty string can be used to not set a proxy, this is the default
13561356
std::string proxyArg = args.GetArg("-proxy", "");
13571357
if (proxyArg != "" && proxyArg != "0") {
1358-
CService proxyAddr;
1359-
if (!Lookup(proxyArg, proxyAddr, 9050, fNameLookup)) {
1358+
const std::optional<CService> proxyAddr{Lookup(proxyArg, 9050, fNameLookup)};
1359+
if (!proxyAddr.has_value()) {
13601360
return InitError(strprintf(_("Invalid -proxy address or hostname: '%s'"), proxyArg));
13611361
}
13621362

1363-
Proxy addrProxy = Proxy(proxyAddr, proxyRandomize);
1363+
Proxy addrProxy = Proxy(proxyAddr.value(), proxyRandomize);
13641364
if (!addrProxy.IsValid())
13651365
return InitError(strprintf(_("Invalid -proxy address or hostname: '%s'"), proxyArg));
13661366

@@ -1386,11 +1386,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
13861386
"reaching the Tor network is explicitly forbidden: -onion=0"));
13871387
}
13881388
} else {
1389-
CService addr;
1390-
if (!Lookup(onionArg, addr, 9050, fNameLookup) || !addr.IsValid()) {
1389+
const std::optional<CService> addr{Lookup(onionArg, 9050, fNameLookup)};
1390+
if (!addr.has_value() || !addr->IsValid()) {
13911391
return InitError(strprintf(_("Invalid -onion address or hostname: '%s'"), onionArg));
13921392
}
1393-
onion_proxy = Proxy{addr, proxyRandomize};
1393+
onion_proxy = Proxy{addr.value(), proxyRandomize};
13941394
}
13951395
}
13961396

@@ -1410,9 +1410,9 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
14101410
}
14111411

14121412
for (const std::string& strAddr : args.GetArgs("-externalip")) {
1413-
CService addrLocal;
1414-
if (Lookup(strAddr, addrLocal, GetListenPort(), fNameLookup) && addrLocal.IsValid())
1415-
AddLocal(addrLocal, LOCAL_MANUAL);
1413+
const std::optional<CService> addrLocal{Lookup(strAddr, GetListenPort(), fNameLookup)};
1414+
if (addrLocal.has_value() && addrLocal->IsValid())
1415+
AddLocal(addrLocal.value(), LOCAL_MANUAL);
14161416
else
14171417
return InitError(ResolveErrMsg("externalip", strAddr));
14181418
}
@@ -1748,22 +1748,24 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
17481748
};
17491749

17501750
for (const std::string& bind_arg : args.GetArgs("-bind")) {
1751-
CService bind_addr;
1751+
std::optional<CService> bind_addr;
17521752
const size_t index = bind_arg.rfind('=');
17531753
if (index == std::string::npos) {
1754-
if (Lookup(bind_arg, bind_addr, default_bind_port, /*fAllowLookup=*/false)) {
1755-
connOptions.vBinds.push_back(bind_addr);
1756-
if (IsBadPort(bind_addr.GetPort())) {
1757-
InitWarning(BadPortWarning("-bind", bind_addr.GetPort()));
1754+
bind_addr = Lookup(bind_arg, default_bind_port, /*fAllowLookup=*/false);
1755+
if (bind_addr.has_value()) {
1756+
connOptions.vBinds.push_back(bind_addr.value());
1757+
if (IsBadPort(bind_addr.value().GetPort())) {
1758+
InitWarning(BadPortWarning("-bind", bind_addr.value().GetPort()));
17581759
}
17591760
continue;
17601761
}
17611762
} else {
17621763
const std::string network_type = bind_arg.substr(index + 1);
17631764
if (network_type == "onion") {
17641765
const std::string truncated_bind_arg = bind_arg.substr(0, index);
1765-
if (Lookup(truncated_bind_arg, bind_addr, BaseParams().OnionServiceTargetPort(), false)) {
1766-
connOptions.onion_binds.push_back(bind_addr);
1766+
bind_addr = Lookup(truncated_bind_arg, BaseParams().OnionServiceTargetPort(), false);
1767+
if (bind_addr.has_value()) {
1768+
connOptions.onion_binds.push_back(bind_addr.value());
17671769
continue;
17681770
}
17691771
}
@@ -1841,11 +1843,11 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
18411843

18421844
const std::string& i2psam_arg = args.GetArg("-i2psam", "");
18431845
if (!i2psam_arg.empty()) {
1844-
CService addr;
1845-
if (!Lookup(i2psam_arg, addr, 7656, fNameLookup) || !addr.IsValid()) {
1846+
const std::optional<CService> addr{Lookup(i2psam_arg, 7656, fNameLookup)};
1847+
if (!addr.has_value() || !addr->IsValid()) {
18461848
return InitError(strprintf(_("Invalid -i2psam address or hostname: '%s'"), i2psam_arg));
18471849
}
1848-
SetProxy(NET_I2P, Proxy{addr});
1850+
SetProxy(NET_I2P, Proxy{addr.value()});
18491851
} else {
18501852
if (args.IsArgSet("-onlynet") && IsReachable(NET_I2P)) {
18511853
return InitError(

‎src/net.cpp

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -130,14 +130,10 @@ uint16_t GetListenPort()
130130
{
131131
// If -bind= is provided with ":port" part, use that (first one if multiple are provided).
132132
for (const std::string& bind_arg : gArgs.GetArgs("-bind")) {
133-
CService bind_addr;
134133
constexpr uint16_t dummy_port = 0;
135134

136-
if (Lookup(bind_arg, bind_addr, dummy_port, /*fAllowLookup=*/false)) {
137-
if (bind_addr.GetPort() != dummy_port) {
138-
return bind_addr.GetPort();
139-
}
140-
}
135+
const std::optional<CService> bind_addr{Lookup(bind_arg, dummy_port, /*fAllowLookup=*/false)};
136+
if (bind_addr.has_value() && bind_addr->GetPort() != dummy_port) return bind_addr->GetPort();
141137
}
142138

143139
// Otherwise, if -whitebind= without NetPermissionFlags::NoBan is provided, use that
@@ -461,9 +457,9 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
461457
const uint16_t default_port{pszDest != nullptr ? Params().GetDefaultPort(pszDest) :
462458
Params().GetDefaultPort()};
463459
if (pszDest) {
464-
std::vector<CService> resolved;
465-
if (Lookup(pszDest, resolved, default_port, fNameLookup && !HaveNameProxy(), 256) && !resolved.empty()) {
466-
const CService rnd{resolved[GetRand(resolved.size())]};
460+
const std::vector<CService> resolved{Lookup(pszDest, default_port, fNameLookup && !HaveNameProxy(), 256)};
461+
if (!resolved.empty()) {
462+
const CService& rnd{resolved[GetRand(resolved.size())]};
467463
addrConnect = CAddress{MaybeFlipIPv6toCJDNS(rnd), NODE_NONE};
468464
if (!addrConnect.IsValid()) {
469465
LogPrint(BCLog::NET, "Resolver returned invalid address %s for %s\n", addrConnect.ToStringAddrPort(), pszDest);

‎src/net_permissions.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -88,18 +88,18 @@ bool NetWhitebindPermissions::TryParse(const std::string& str, NetWhitebindPermi
8888
if (!TryParsePermissionFlags(str, flags, offset, error)) return false;
8989

9090
const std::string strBind = str.substr(offset);
91-
CService addrBind;
92-
if (!Lookup(strBind, addrBind, 0, false)) {
91+
const std::optional<CService> addrBind{Lookup(strBind, 0, false)};
92+
if (!addrBind.has_value()) {
9393
error = ResolveErrMsg("whitebind", strBind);
9494
return false;
9595
}
96-
if (addrBind.GetPort() == 0) {
96+
if (addrBind.value().GetPort() == 0) {
9797
error = strprintf(_("Need to specify a port with -whitebind: '%s'"), strBind);
9898
return false;
9999
}
100100

101101
output.m_flags = flags;
102-
output.m_service = addrBind;
102+
output.m_service = addrBind.value();
103103
error = Untranslated("");
104104
return true;
105105
}

‎src/netbase.cpp

Lines changed: 13 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -185,48 +185,39 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSL
185185
return true;
186186
}
187187

188-
bool Lookup(const std::string& name, std::vector<CService>& vAddr, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function)
188+
std::vector<CService> Lookup(const std::string& name, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function)
189189
{
190190
if (name.empty() || !ContainsNoNUL(name)) {
191-
return false;
191+
return {};
192192
}
193193
uint16_t port{portDefault};
194194
std::string hostname;
195195
SplitHostPort(name, port, hostname);
196196

197197
const std::vector<CNetAddr> addresses{LookupIntern(hostname, nMaxSolutions, fAllowLookup, dns_lookup_function)};
198-
if (addresses.empty())
199-
return false;
200-
vAddr.resize(addresses.size());
201-
for (unsigned int i = 0; i < addresses.size(); i++)
202-
vAddr[i] = CService(addresses[i], port);
203-
return true;
198+
if (addresses.empty()) return {};
199+
std::vector<CService> services;
200+
services.reserve(addresses.size());
201+
for (const auto& addr : addresses)
202+
services.emplace_back(addr, port);
203+
return services;
204204
}
205205

206-
bool Lookup(const std::string& name, CService& addr, uint16_t portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function)
206+
std::optional<CService> Lookup(const std::string& name, uint16_t portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function)
207207
{
208-
if (!ContainsNoNUL(name)) {
209-
return false;
210-
}
211-
std::vector<CService> vService;
212-
bool fRet = Lookup(name, vService, portDefault, fAllowLookup, 1, dns_lookup_function);
213-
if (!fRet)
214-
return false;
215-
addr = vService[0];
216-
return true;
208+
const std::vector<CService> services{Lookup(name, portDefault, fAllowLookup, 1, dns_lookup_function)};
209+
210+
return services.empty() ? std::nullopt : std::make_optional(services.front());
217211
}
218212

219213
CService LookupNumeric(const std::string& name, uint16_t portDefault, DNSLookupFn dns_lookup_function)
220214
{
221215
if (!ContainsNoNUL(name)) {
222216
return {};
223217
}
224-
CService addr;
225218
// "1.2:345" will fail to resolve the ip, but will still set the port.
226219
// If the ip fails to resolve, re-init the result.
227-
if(!Lookup(name, addr, portDefault, false, dns_lookup_function))
228-
addr = CService();
229-
return addr;
220+
return Lookup(name, portDefault, /*fAllowLookup=*/false, dns_lookup_function).value_or(CService{});
230221
}
231222

232223
/** SOCKS version */

‎src/netbase.h

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ extern DNSLookupFn g_dns_lookup;
109109
* @returns The resulting network addresses to which the specified host
110110
* string resolved.
111111
*
112-
* @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn)
112+
* @see Lookup(const std::string&, uint16_t, bool, unsigned int, DNSLookupFn)
113113
* for additional parameter descriptions.
114114
*/
115115
std::vector<CNetAddr> LookupHost(const std::string& name, unsigned int nMaxSolutions, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup);
@@ -133,35 +133,33 @@ bool LookupHost(const std::string& name, CNetAddr& addr, bool fAllowLookup, DNSL
133133
* disambiguated bracketed form), optionally followed by a uint16_t port
134134
* number. (e.g. example.com:8333 or
135135
* [2001:db8:85a3:8d3:1319:8a2e:370:7348]:420)
136-
* @param[out] vAddr The resulting services to which the specified service string
137-
* resolved.
138136
* @param portDefault The default port for resulting services if not specified
139137
* by the service string.
140138
* @param fAllowLookup Whether or not hostname lookups are permitted. If yes,
141139
* external queries may be performed.
142140
* @param nMaxSolutions The maximum number of results we want, specifying 0
143141
* means "as many solutions as we get."
144142
*
145-
* @returns Whether or not the service string successfully resolved to any
146-
* resulting services.
143+
* @returns The resulting services to which the specified service string
144+
* resolved.
147145
*/
148-
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);
146+
std::vector<CService> Lookup(const std::string& name, uint16_t portDefault, bool fAllowLookup, unsigned int nMaxSolutions, DNSLookupFn dns_lookup_function = g_dns_lookup);
149147

150148
/**
151149
* Resolve a service string to its first corresponding service.
152150
*
153-
* @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn)
151+
* @see Lookup(const std::string&, uint16_t, bool, unsigned int, DNSLookupFn)
154152
* for additional parameter descriptions.
155153
*/
156-
bool Lookup(const std::string& name, CService& addr, uint16_t portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup);
154+
std::optional<CService> Lookup(const std::string& name, uint16_t portDefault, bool fAllowLookup, DNSLookupFn dns_lookup_function = g_dns_lookup);
157155

158156
/**
159157
* Resolve a service string with a numeric IP to its first corresponding
160158
* service.
161159
*
162160
* @returns The resulting CService if the resolution was successful, [::]:0 otherwise.
163161
*
164-
* @see Lookup(const std::string&, std::vector<CService>&, uint16_t, bool, unsigned int, DNSLookupFn)
162+
* @see Lookup(const std::string&, uint16_t, bool, unsigned int, DNSLookupFn)
165163
* for additional parameter descriptions.
166164
*/
167165
CService LookupNumeric(const std::string& name, uint16_t portDefault = 0, DNSLookupFn dns_lookup_function = g_dns_lookup);

‎src/test/addrman_tests.cpp

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ static CNetAddr ResolveIP(const std::string& ip)
4040

4141
static CService ResolveService(const std::string& ip, uint16_t port = 0)
4242
{
43-
CService serv;
44-
BOOST_CHECK_MESSAGE(Lookup(ip, serv, port, false), strprintf("failed to resolve: %s:%i", ip, port));
45-
return serv;
43+
const std::optional<CService> serv{Lookup(ip, port, false)};
44+
BOOST_CHECK_MESSAGE(serv.has_value(), strprintf("failed to resolve: %s:%i", ip, port));
45+
return serv.value_or(CService{});
4646
}
4747

4848

@@ -948,18 +948,23 @@ BOOST_AUTO_TEST_CASE(load_addrman)
948948
{
949949
AddrMan addrman{EMPTY_NETGROUPMAN, DETERMINISTIC, GetCheckRatio(m_node)};
950950

951-
CService addr1, addr2, addr3;
952-
BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false));
953-
BOOST_CHECK(Lookup("250.7.2.2", addr2, 9999, false));
954-
BOOST_CHECK(Lookup("250.7.3.3", addr3, 9999, false));
955-
BOOST_CHECK(Lookup("250.7.3.3"s, addr3, 9999, false));
956-
BOOST_CHECK(!Lookup("250.7.3.3\0example.com"s, addr3, 9999, false));
951+
std::optional<CService> addr1, addr2, addr3, addr4;
952+
addr1 = Lookup("250.7.1.1", 8333, false);
953+
BOOST_CHECK(addr1.has_value());
954+
addr2 = Lookup("250.7.2.2", 9999, false);
955+
BOOST_CHECK(addr2.has_value());
956+
addr3 = Lookup("250.7.3.3", 9999, false);
957+
BOOST_CHECK(addr3.has_value());
958+
addr3 = Lookup("250.7.3.3"s, 9999, false);
959+
BOOST_CHECK(addr3.has_value());
960+
addr4 = Lookup("250.7.3.3\0example.com"s, 9999, false);
961+
BOOST_CHECK(!addr4.has_value());
957962

958963
// Add three addresses to new table.
959-
CService source;
960-
BOOST_CHECK(Lookup("252.5.1.1", source, 8333, false));
961-
std::vector<CAddress> addresses{CAddress(addr1, NODE_NONE), CAddress(addr2, NODE_NONE), CAddress(addr3, NODE_NONE)};
962-
BOOST_CHECK(addrman.Add(addresses, source));
964+
const std::optional<CService> source{Lookup("252.5.1.1", 8333, false)};
965+
BOOST_CHECK(source.has_value());
966+
std::vector<CAddress> addresses{CAddress(addr1.value(), NODE_NONE), CAddress(addr2.value(), NODE_NONE), CAddress(addr3.value(), NODE_NONE)};
967+
BOOST_CHECK(addrman.Add(addresses, source.value()));
963968
BOOST_CHECK(addrman.Size() == 3);
964969

965970
// Test that the de-serialization does not throw an exception.
@@ -1004,11 +1009,11 @@ static CDataStream MakeCorruptPeersDat()
10041009
int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30);
10051010
s << nUBuckets;
10061011

1007-
CService serv;
1008-
BOOST_CHECK(Lookup("252.1.1.1", serv, 7777, false));
1009-
CAddress addr = CAddress(serv, NODE_NONE);
1012+
const std::optional<CService> serv{Lookup("252.1.1.1", 7777, false)};
1013+
BOOST_REQUIRE(serv.has_value());
1014+
CAddress addr = CAddress(serv.value(), NODE_NONE);
10101015
CNetAddr resolved;
1011-
BOOST_CHECK(LookupHost("252.2.2.2", resolved, false));
1016+
BOOST_REQUIRE(LookupHost("252.2.2.2", resolved, false));
10121017
AddrInfo info = AddrInfo(addr, resolved);
10131018
s << info;
10141019

‎src/test/fuzz/netbase_dns_lookup.cpp

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -42,18 +42,16 @@ FUZZ_TARGET(netbase_dns_lookup)
4242
}
4343
}
4444
{
45-
std::vector<CService> resolved_services;
46-
if (Lookup(name, resolved_services, default_port, allow_lookup, max_results, fuzzed_dns_lookup_function)) {
47-
for (const CNetAddr& resolved_service : resolved_services) {
48-
assert(!resolved_service.IsInternal());
49-
}
45+
const std::vector<CService> resolved_services{Lookup(name, default_port, allow_lookup, max_results, fuzzed_dns_lookup_function)};
46+
for (const CNetAddr& resolved_service : resolved_services) {
47+
assert(!resolved_service.IsInternal());
5048
}
5149
assert(resolved_services.size() <= max_results || max_results == 0);
5250
}
5351
{
54-
CService resolved_service;
55-
if (Lookup(name, resolved_service, default_port, allow_lookup, fuzzed_dns_lookup_function)) {
56-
assert(!resolved_service.IsInternal());
52+
const std::optional<CService> resolved_service{Lookup(name, default_port, allow_lookup, fuzzed_dns_lookup_function)};
53+
if (resolved_service.has_value()) {
54+
assert(!resolved_service.value().IsInternal());
5755
}
5856
}
5957
{

‎src/torcontrol.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,15 @@ bool TorControlConnection::Connect(const std::string& tor_control_center, const
133133
Disconnect();
134134
}
135135

136-
CService control_service;
137-
if (!Lookup(tor_control_center, control_service, 9051, fNameLookup)) {
136+
const std::optional<CService> control_service{Lookup(tor_control_center, 9051, fNameLookup)};
137+
if (!control_service.has_value()) {
138138
LogPrintf("tor: Failed to look up control center %s\n", tor_control_center);
139139
return false;
140140
}
141141

142142
struct sockaddr_storage control_address;
143143
socklen_t control_address_len = sizeof(control_address);
144-
if (!control_service.GetSockAddr(reinterpret_cast<struct sockaddr*>(&control_address), &control_address_len)) {
144+
if (!control_service.value().GetSockAddr(reinterpret_cast<struct sockaddr*>(&control_address), &control_address_len)) {
145145
LogPrintf("tor: Error parsing socket address %s\n", tor_control_center);
146146
return false;
147147
}

0 commit comments

Comments
 (0)