Skip to content

Commit

Permalink
Merge bitcoin#25619: net: avoid overriding non-virtual ToString() in …
Browse files Browse the repository at this point in the history 
…CService and use better naming

c9d548c net: remove CService::ToStringPort() (Vasil Dimov)
fd4f0f4 gui: simplify OptionsDialog::updateDefaultProxyNets() (Vasil Dimov)
96c791d net: remove CService::ToString() use ToStringAddrPort() instead (Vasil Dimov)
944a9de net: remove CNetAddr::ToString() and use ToStringAddr() instead (Vasil Dimov)
043b9de scripted-diff: rename ToStringIP[Port]() to ToStringAddr[Port]() (Vasil Dimov)

Pull request description:

  Before this PR we had the somewhat confusing combination of methods:

  `CNetAddr::ToStringIP()`
  `CNetAddr::ToString()` (duplicate of the above)
  `CService::ToStringIPPort()`
  `CService::ToString()` (duplicate of the above, overrides a non-virtual method from `CNetAddr`)
  `CService::ToStringPort()`

  Avoid [overriding non-virtual methods](bitcoin#25349).

  "IP" stands for "Internet Protocol" and while sometimes "IP addresses" are called just "IPs", it is incorrect to call Tor or I2P addresses "IPs". Thus use "Addr" instead of "IP".

  Change the above to:

  `CNetAddr::ToStringAddr()`
  `CService::ToStringAddrPort()`

  The changes touch a lot of files, but are mostly mechanical.

ACKs for top commit:
  sipa:
    utACK c9d548c
  achow101:
    ACK c9d548c
  jonatack:
    re-ACK c9d548c only change since my previous reviews is rebase, but as a sanity check rebased to current master and at each commit quickly re-reviewed and re-verified clean build and green unit tests
  LarryRuane:
    ACK c9d548c

Tree-SHA512: 633fb044bdecf9f551b5e3314c385bf10e2b78e8027dc51ec324b66b018da35e5b01f3fbe6295bbc455ea1bcd1a3629de1918d28de510693afaf6a52693f2157
  • Loading branch information
@achow101
achow101 committed Feb 17, 2023
2 parents 27772d8 + c9d548c commit 35fbc97
Show file tree
Hide file tree
Showing 20 changed files with 138 additions and 160 deletions.
22 changes: 11 additions & 11 deletions src/addrman.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ void AddrManImpl::ClearNew(int nUBucket, int nUBucketPos)
assert(infoDelete.nRefCount > 0);
infoDelete.nRefCount--;
vvNew[nUBucket][nUBucketPos] = -1;
LogPrint(BCLog::ADDRMAN, "Removed %s from new[%i][%i]\n", infoDelete.ToString(), nUBucket, nUBucketPos);
LogPrint(BCLog::ADDRMAN, "Removed %s from new[%i][%i]\n", infoDelete.ToStringAddrPort(), nUBucket, nUBucketPos);
if (infoDelete.nRefCount == 0) {
Delete(nIdDelete);
}
Expand Down Expand Up @@ -542,7 +542,7 @@ void AddrManImpl::MakeTried(AddrInfo& info, int nId)
nNew++;
m_network_counts[infoOld.GetNetwork()].n_new++;
LogPrint(BCLog::ADDRMAN, "Moved %s from tried[%i][%i] to new[%i][%i] to make space\n",
infoOld.ToString(), nKBucket, nKBucketPos, nUBucket, nUBucketPos);
infoOld.ToStringAddrPort(), nKBucket, nKBucketPos, nUBucket, nUBucketPos);
}
assert(vvTried[nKBucket][nKBucketPos] == -1);

Expand Down Expand Up @@ -618,7 +618,7 @@ bool AddrManImpl::AddSingle(const CAddress& addr, const CNetAddr& source, std::c
pinfo->nRefCount++;
vvNew[nUBucket][nUBucketPos] = nId;
LogPrint(BCLog::ADDRMAN, "Added %s mapped to AS%i to new[%i][%i]\n",
addr.ToString(), m_netgroupman.GetMappedAS(addr), nUBucket, nUBucketPos);
addr.ToStringAddrPort(), m_netgroupman.GetMappedAS(addr), nUBucket, nUBucketPos);
} else {
if (pinfo->nRefCount == 0) {
Delete(nId);
Expand Down Expand Up @@ -669,15 +669,15 @@ bool AddrManImpl::Good_(const CService& addr, bool test_before_evict, NodeSecond
// Output the entry we'd be colliding with, for debugging purposes
auto colliding_entry = mapInfo.find(vvTried[tried_bucket][tried_bucket_pos]);
LogPrint(BCLog::ADDRMAN, "Collision with %s while attempting to move %s to tried table. Collisions=%d\n",
colliding_entry != mapInfo.end() ? colliding_entry->second.ToString() : "",
addr.ToString(),
colliding_entry != mapInfo.end() ? colliding_entry->second.ToStringAddrPort() : "",
addr.ToStringAddrPort(),
m_tried_collisions.size());
return false;
} else {
// move nId to the tried tables
MakeTried(info, nId);
LogPrint(BCLog::ADDRMAN, "Moved %s mapped to AS%i to tried[%i][%i]\n",
addr.ToString(), m_netgroupman.GetMappedAS(addr), tried_bucket, tried_bucket_pos);
addr.ToStringAddrPort(), m_netgroupman.GetMappedAS(addr), tried_bucket, tried_bucket_pos);
return true;
}
}
Expand All @@ -689,7 +689,7 @@ bool AddrManImpl::Add_(const std::vector<CAddress>& vAddr, const CNetAddr& sourc
added += AddSingle(*it, source, time_penalty) ? 1 : 0;
}
if (added > 0) {
LogPrint(BCLog::ADDRMAN, "Added %i addresses (of %i) from %s: %i tried, %i new\n", added, vAddr.size(), source.ToString(), nTried, nNew);
LogPrint(BCLog::ADDRMAN, "Added %i addresses (of %i) from %s: %i tried, %i new\n", added, vAddr.size(), source.ToStringAddr(), nTried, nNew);
}
return added > 0;
}
Expand Down Expand Up @@ -746,7 +746,7 @@ std::pair<CAddress, NodeSeconds> AddrManImpl::Select_(bool newOnly) const
const AddrInfo& info{it_found->second};
// With probability GetChance() * fChanceFactor, return the entry.
if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) {
LogPrint(BCLog::ADDRMAN, "Selected %s from tried\n", info.ToString());
LogPrint(BCLog::ADDRMAN, "Selected %s from tried\n", info.ToStringAddrPort());
return {info, info.m_last_try};
}
// Otherwise start over with a (likely) different bucket, and increased chance factor.
Expand Down Expand Up @@ -774,7 +774,7 @@ std::pair<CAddress, NodeSeconds> AddrManImpl::Select_(bool newOnly) const
const AddrInfo& info{it_found->second};
// With probability GetChance() * fChanceFactor, return the entry.
if (insecure_rand.randbits(30) < fChanceFactor * info.GetChance() * (1 << 30)) {
LogPrint(BCLog::ADDRMAN, "Selected %s from new\n", info.ToString());
LogPrint(BCLog::ADDRMAN, "Selected %s from new\n", info.ToStringAddrPort());
return {info, info.m_last_try};
}
// Otherwise start over with a (likely) different bucket, and increased chance factor.
Expand Down Expand Up @@ -891,7 +891,7 @@ void AddrManImpl::ResolveCollisions_()

// Give address at least 60 seconds to successfully connect
if (current_time - info_old.m_last_try > 60s) {
LogPrint(BCLog::ADDRMAN, "Replacing %s with %s in tried table\n", info_old.ToString(), info_new.ToString());
LogPrint(BCLog::ADDRMAN, "Replacing %s with %s in tried table\n", info_old.ToStringAddrPort(), info_new.ToStringAddrPort());

// Replaces an existing address already in the tried table with the new address
Good_(info_new, false, current_time);
Expand All @@ -901,7 +901,7 @@ void AddrManImpl::ResolveCollisions_()
// If the collision hasn't resolved in some reasonable amount of time,
// just evict the old entry -- we must not be able to
// connect to it for some reason.
LogPrint(BCLog::ADDRMAN, "Unable to test; replacing %s with %s in tried table anyway\n", info_old.ToString(), info_new.ToString());
LogPrint(BCLog::ADDRMAN, "Unable to test; replacing %s with %s in tried table anyway\n", info_old.ToStringAddrPort(), info_new.ToStringAddrPort());
Good_(info_new, false, current_time);
erase_collision = true;
}
Expand Down
2 changes: 1 addition & 1 deletion src/httprpc.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)

JSONRPCRequest jreq;
jreq.context = context;
jreq.peerAddr = req->GetPeer().ToString();
jreq.peerAddr = req->GetPeer().ToStringAddrPort();
if (!RPCAuthorized(authHeader.second, jreq.authUser)) {
LogPrintf("ThreadRPCServer incorrect password attempt from %s\n", jreq.peerAddr);

Expand Down
6 changes: 3 additions & 3 deletions src/httpserver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,21 +222,21 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
// Early address-based allow check
if (!ClientAllowed(hreq->GetPeer())) {
LogPrint(BCLog::HTTP, "HTTP request from %s rejected: Client network is not allowed RPC access\n",
hreq->GetPeer().ToString());
hreq->GetPeer().ToStringAddrPort());
hreq->WriteReply(HTTP_FORBIDDEN);
return;
}

// Early reject unknown HTTP methods
if (hreq->GetRequestMethod() == HTTPRequest::UNKNOWN) {
LogPrint(BCLog::HTTP, "HTTP request from %s rejected: Unknown HTTP request method\n",
hreq->GetPeer().ToString());
hreq->GetPeer().ToStringAddrPort());
hreq->WriteReply(HTTP_BAD_METHOD);
return;
}

LogPrint(BCLog::HTTP, "Received a %s request for %s from %s\n",
RequestMethodString(hreq->GetRequestMethod()), SanitizeString(hreq->GetURI(), SAFE_CHARS_URI).substr(0, 100), hreq->GetPeer().ToString());
RequestMethodString(hreq->GetRequestMethod()), SanitizeString(hreq->GetURI(), SAFE_CHARS_URI).substr(0, 100), hreq->GetPeer().ToStringAddrPort());

// Find registered handler for prefix
std::string strURI = hreq->GetURI();
Expand Down
10 changes: 5 additions & 5 deletions src/i2p.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)
}

const Reply& lookup_reply =
SendRequestAndGetReply(*sock, strprintf("NAMING LOOKUP NAME=%s", to.ToStringIP()));
SendRequestAndGetReply(*sock, strprintf("NAMING LOOKUP NAME=%s", to.ToStringAddr()));

const std::string& dest = lookup_reply.Get("VALUE");

Expand All @@ -233,7 +233,7 @@ bool Session::Connect(const CService& to, Connection& conn, bool& proxy_error)

throw std::runtime_error(strprintf("\"%s\"", connect_reply.full));
} catch (const std::runtime_error& e) {
Log("Error connecting to %s: %s", to.ToString(), e.what());
Log("Error connecting to %s: %s", to.ToStringAddrPort(), e.what());
CheckControlSock();
return false;
}
Expand Down Expand Up @@ -302,7 +302,7 @@ std::unique_ptr<Sock> Session::Hello() const
}

if (!ConnectSocketDirectly(m_control_host, *sock, nConnectTimeout, true)) {
throw std::runtime_error(strprintf("Cannot connect to %s", m_control_host.ToString()));
throw std::runtime_error(strprintf("Cannot connect to %s", m_control_host.ToStringAddrPort()));
}

SendRequestAndGetReply(*sock, "HELLO VERSION MIN=3.1 MAX=3.1");
Expand Down Expand Up @@ -371,7 +371,7 @@ void Session::CreateIfNotCreatedAlready()
const auto session_type = m_transient ? "transient" : "persistent";
const auto session_id = GetRandHash().GetHex().substr(0, 10); // full is overkill, too verbose in the logs

Log("Creating %s SAM session %s with %s", session_type, session_id, m_control_host.ToString());
Log("Creating %s SAM session %s with %s", session_type, session_id, m_control_host.ToStringAddrPort());

auto sock = Hello();

Expand Down Expand Up @@ -408,7 +408,7 @@ void Session::CreateIfNotCreatedAlready()
Log("%s SAM session %s created, my address=%s",
Capitalize(session_type),
m_session_id,
m_my_addr.ToString());
m_my_addr.ToStringAddrPort());
}

std::unique_ptr<Sock> Session::StreamAccept()
Expand Down
2 changes: 1 addition & 1 deletion src/init.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1798,7 +1798,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info)
if (connOptions.onion_binds.size() > 1) {
InitWarning(strprintf(_("More than one onion bind address is provided. Using %s "
"for the automatically created Tor onion service."),
onion_service_target.ToStringIPPort()));
onion_service_target.ToStringAddrPort()));
}
StartTorControl(onion_service_target);
}
Expand Down
4 changes: 2 additions & 2 deletions src/mapport.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ static bool NatpmpMapping(natpmp_t* natpmp, const struct in_addr& external_ipv4_
AddLocal(external, LOCAL_MAPPED);
external_ip_discovered = true;
}
LogPrintf("natpmp: Port mapping successful. External address = %s\n", external.ToString());
LogPrintf("natpmp: Port mapping successful. External address = %s\n", external.ToStringAddrPort());
return true;
} else {
LogPrintf("natpmp: Port mapping failed.\n");
Expand Down Expand Up @@ -177,7 +177,7 @@ static bool ProcessUpnp()
if (externalIPAddress[0]) {
CNetAddr resolved;
if (LookupHost(externalIPAddress, resolved, false)) {
LogPrintf("UPnP: ExternalIPAddress = %s\n", resolved.ToString());
LogPrintf("UPnP: ExternalIPAddress = %s\n", resolved.ToStringAddr());
AddLocal(resolved, LOCAL_MAPPED);
}
} else {
Expand Down
Loading

0 comments on commit 35fbc97

Please sign in to comment.