Merge bitcoin#22782: Remove unused MaybeSetAddrName

fa9eade Remove GetAddrName (MarcoFalke) fa78657 Remove unused RecursiveMutex cs_addrName (MarcoFalke) fa82f4e Remove unused MaybeSetAddrName (MarcoFalke) Pull request description: . ACKs for top commit: jnewbery: Code review ACK fa9eade naumenkogs: utACK fa9eade Tree-SHA512: 61501a699add59225dc8127b6dfdda450d768c86f958fdf94e9c28309c3705ecfbee4b064d44228b8c1190c19c39272becc7ede8386ac1406699ea2285881c72
hebasto · Aug 27, 2021 · 33707a2 · 33707a2
2 parents 19364c0 + fa9eade
commit 33707a2
Show file tree

Hide file tree

Showing 9 changed files with 19 additions and 44 deletions.
diff --git a/doc/tracing.md b/doc/tracing.md
@@ -147,7 +147,7 @@ For example:
 ```C++
 TRACE6(net, inbound_message,
     pnode->GetId(),
-    pnode->GetAddrName().c_str(),
+    pnode->m_addr_name.c_str(),
     pnode->ConnectionTypeAsString().c_str(),
     sanitizedType.c_str(),
     msg.data.size(),

diff --git a/src/net.cpp b/src/net.cpp
@@ -331,7 +331,7 @@ CNode* CConnman::FindNode(const std::string& addrName)
 {
     LOCK(cs_vNodes);
     for (CNode* pnode : vNodes) {
-        if (pnode->GetAddrName() == addrName) {
+        if (pnode->m_addr_name == addrName) {
             return pnode;
         }
     }
@@ -414,14 +414,10 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
                 return nullptr;
             }
             // It is possible that we already have a connection to the IP/port pszDest resolved to.
-            // In that case, drop the connection that was just created, and return the existing CNode instead.
-            // Also store the name we used to connect in that CNode, so that future FindNode() calls to that
-            // name catch this early.
+            // In that case, drop the connection that was just created.
             LOCK(cs_vNodes);
             CNode* pnode = FindNode(static_cast<CService>(addrConnect));
-            if (pnode)
-            {
-                pnode->MaybeSetAddrName(std::string(pszDest));
+            if (pnode) {
                 LogPrintf("Failed to open new connection, already connected\n");
                 return nullptr;
             }
@@ -534,19 +530,8 @@ std::string ConnectionTypeAsString(ConnectionType conn_type)
     assert(false);
 }
 
-std::string CNode::GetAddrName() const {
-    LOCK(cs_addrName);
-    return addrName;
-}
-
-void CNode::MaybeSetAddrName(const std::string& addrNameIn) {
-    LOCK(cs_addrName);
-    if (addrName.empty()) {
-        addrName = addrNameIn;
-    }
-}
-
-CService CNode::GetAddrLocal() const {
+CService CNode::GetAddrLocal() const
+{
     LOCK(cs_addrLocal);
     return addrLocal;
 }
@@ -587,7 +572,7 @@ void CNode::copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap)
     X(nLastBlockTime);
     X(nTimeConnected);
     X(nTimeOffset);
-    stats.addrName = GetAddrName();
+    X(m_addr_name);
     X(nVersion);
     {
         LOCK(cs_SubVer);
@@ -2137,7 +2122,7 @@ std::vector<AddedNodeInfo> CConnman::GetAddedNodeInfo() const
             if (pnode->addr.IsValid()) {
                 mapConnected[pnode->addr] = pnode->IsInboundConn();
             }
-            std::string addrName = pnode->GetAddrName();
+            std::string addrName{pnode->m_addr_name};
             if (!addrName.empty()) {
                 mapConnectedByName[std::move(addrName)] = std::make_pair(pnode->IsInboundConn(), static_cast<const CService&>(pnode->addr));
             }
@@ -2966,6 +2951,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const
     : nTimeConnected(GetTimeSeconds()),
       addr(addrIn),
       addrBind(addrBindIn),
+      m_addr_name{addrNameIn.empty() ? addr.ToStringIPPort() : addrNameIn},
       m_inbound_onion(inbound_onion),
       nKeyedNetGroup(nKeyedNetGroupIn),
       id(idIn),
@@ -2975,7 +2961,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const
 {
     if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND);
     hSocket = hSocketIn;
-    addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn;
     if (conn_type_in != ConnectionType::BLOCK_RELAY) {
         m_tx_relay = std::make_unique<TxRelay>();
     }
@@ -2985,7 +2970,7 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const
     mapRecvBytesPerMsgCmd[NET_MESSAGE_COMMAND_OTHER] = 0;
 
     if (fLogIPs) {
-        LogPrint(BCLog::NET, "Added connection to %s peer=%d\n", addrName, id);
+        LogPrint(BCLog::NET, "Added connection to %s peer=%d\n", m_addr_name, id);
     } else {
         LogPrint(BCLog::NET, "Added connection peer=%d\n", id);
     }
@@ -3014,7 +2999,7 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg)
 
     TRACE6(net, outbound_message,
         pnode->GetId(),
-        pnode->GetAddrName().c_str(),
+        pnode->m_addr_name.c_str(),
         pnode->ConnectionTypeAsString().c_str(),
         msg.m_type.c_str(),
         msg.data.size(),

diff --git a/src/net.h b/src/net.h
@@ -248,7 +248,7 @@ class CNodeStats
     int64_t nLastBlockTime;
     int64_t nTimeConnected;
     int64_t nTimeOffset;
-    std::string addrName;
+    std::string m_addr_name;
     int nVersion;
     std::string cleanSubVer;
     bool fInbound;
@@ -430,6 +430,7 @@ class CNode
     const CAddress addr;
     // Bind address of our side of the connection
     const CAddress addrBind;
+    const std::string m_addr_name;
     //! Whether this peer is an inbound onion, i.e. connected via our Tor onion service.
     const bool m_inbound_onion;
     std::atomic<int> nVersion{0};
@@ -658,10 +659,6 @@ class CNode
         return nLocalServices;
     }
 
-    std::string GetAddrName() const;
-    //! Sets the addrName only if it was not previously set
-    void MaybeSetAddrName(const std::string& addrNameIn);
-
     std::string ConnectionTypeAsString() const { return ::ConnectionTypeAsString(m_conn_type); }
 
     /** A ping-pong round trip has completed successfully. Update latest and minimum ping times. */
@@ -693,10 +690,7 @@ class CNode
     //! service advertisements.
     const ServiceFlags nLocalServices;
 
-    std::list<CNetMessage> vRecvMsg;  // Used only by SocketHandler thread
-
-    mutable RecursiveMutex cs_addrName;
-    std::string addrName GUARDED_BY(cs_addrName);
+    std::list<CNetMessage> vRecvMsg; // Used only by SocketHandler thread
 
     // Our address, as reported by the peer
     CService addrLocal GUARDED_BY(cs_addrLocal);

diff --git a/src/net_processing.cpp b/src/net_processing.cpp
@@ -4086,7 +4086,7 @@ bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt
 
     TRACE6(net, inbound_message,
         pfrom->GetId(),
-        pfrom->GetAddrName().c_str(),
+        pfrom->m_addr_name.c_str(),
         pfrom->ConnectionTypeAsString().c_str(),
         msg.m_command.c_str(),
         msg.m_recv.size(),

diff --git a/src/qt/peertablemodel.cpp b/src/qt/peertablemodel.cpp
@@ -72,7 +72,7 @@ QVariant PeerTableModel::data(const QModelIndex& index, int role) const
         case NetNodeId:
             return (qint64)rec->nodeStats.nodeid;
         case Address:
-            return QString::fromStdString(rec->nodeStats.addrName);
+            return QString::fromStdString(rec->nodeStats.m_addr_name);
         case Direction:
             return QString(rec->nodeStats.fInbound ?
                                //: An Inbound Connection from a Peer.

diff --git a/src/qt/peertablesortproxy.cpp b/src/qt/peertablesortproxy.cpp
@@ -25,7 +25,7 @@ bool PeerTableSortProxy::lessThan(const QModelIndex& left_index, const QModelInd
     case PeerTableModel::NetNodeId:
         return left_stats.nodeid < right_stats.nodeid;
     case PeerTableModel::Address:
-        return left_stats.addrName.compare(right_stats.addrName) < 0;
+        return left_stats.m_addr_name.compare(right_stats.m_addr_name) < 0;
     case PeerTableModel::Direction:
         return left_stats.fInbound > right_stats.fInbound; // default sort Inbound, then Outbound
     case PeerTableModel::ConnectionType:

diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
@@ -1136,7 +1136,7 @@ void RPCConsole::updateDetailWidget()
     }
     const auto stats = selected_peers.first().data(PeerTableModel::StatsRole).value<CNodeCombinedStats*>();
     // update the detail ui with latest node information
-    QString peerAddrDetails(QString::fromStdString(stats->nodeStats.addrName) + " ");
+    QString peerAddrDetails(QString::fromStdString(stats->nodeStats.m_addr_name) + " ");
     peerAddrDetails += tr("(peer: %1)").arg(QString::number(stats->nodeStats.nodeid));
     if (!stats->nodeStats.addrLocal.empty())
         peerAddrDetails += "<br />" + tr("via %1").arg(QString::fromStdString(stats->nodeStats.addrLocal));

diff --git a/src/rpc/net.cpp b/src/rpc/net.cpp
@@ -197,7 +197,7 @@ static RPCHelpMan getpeerinfo()
         CNodeStateStats statestats;
         bool fStateStats = peerman.GetNodeStateStats(stats.nodeid, statestats);
         obj.pushKV("id", stats.nodeid);
-        obj.pushKV("addr", stats.addrName);
+        obj.pushKV("addr", stats.m_addr_name);
         if (stats.addrBind.IsValid()) {
             obj.pushKV("addrbind", stats.addrBind.ToString());
         }

diff --git a/src/test/fuzz/net.cpp b/src/test/fuzz/net.cpp
@@ -37,9 +37,6 @@ FUZZ_TARGET_INIT(net, initialize_net)
             [&] {
                 node.CloseSocketDisconnect();
             },
-            [&] {
-                node.MaybeSetAddrName(fuzzed_data_provider.ConsumeRandomLengthString(32));
-            },
             [&] {
                 const std::vector<bool> asmap = ConsumeRandomLengthBitVector(fuzzed_data_provider);
                 if (!SanityCheckASMap(asmap)) {
@@ -82,7 +79,6 @@ FUZZ_TARGET_INIT(net, initialize_net)
     }
 
     (void)node.GetAddrLocal();
-    (void)node.GetAddrName();
     (void)node.GetId();
     (void)node.GetLocalNonce();
     (void)node.GetLocalServices();