Merge bitcoin/bitcoin#23219: p2p, refactor: tidy up LookupSubNet()

c44c20108f7b7314f59f034110789385a24db280 p2p, refactor: drop unused DNSLookupFn param in LookupSubnet() (Vasil Dimov)
f0c9e68080432c1ab11b14e571b8dfb7cfe727f8 p2p, refactor: tidy up LookupSubNet() (Jon Atack)

Pull request description:

  This pull originally resolved a code `TO-DO`, as well as fixing different param names between the function declaration and definition, updating the function to current style standards, clearer variable naming, and improving the Doxygen documentation.

  Following the merge of #17160, it now does the non-`TODO` changes and also now drops an unused param to simplify the function.

ACKs for top commit:
  dunxen:
    ACK c44c201
  vasild:
    ACK c44c20108f7b7314f59f034110789385a24db280
  shaavan:
    crACK c44c20108f7b7314f59f034110789385a24db280

Tree-SHA512: 55f64c7f403819dec84f4da06e63db50f7c0601a2d9a1ec196fda667c220ec6f5ad2a3c95e0e02275da9f6da6b984275d1dc10e19ed82005c5e13da5c5ecab02

3 files changed, 32 insertions, 35 deletions

diff --git a/src/netbase.cpp b/src/netbase.cpp
index 3cb12f1abc..0b68c0d041 100644
--- a/src/netbase.cpp
+++ b/src/netbase.cpp
@@ -676,40 +676,36 @@ bool ConnectThroughProxy(const proxyType& proxy, const std::string& strDest, uin
     return true;
 }
 
-bool LookupSubNet(const std::string& strSubnet, CSubNet& ret, DNSLookupFn dns_lookup_function)
+bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out)
 {
-    if (!ValidAsCString(strSubnet)) {
+    if (!ValidAsCString(subnet_str)) {
         return false;
     }
-    size_t slash = strSubnet.find_last_of('/');
-    CNetAddr network;
 
-    std::string strAddress = strSubnet.substr(0, slash);
-    if (LookupHost(strAddress, network, false, dns_lookup_function))
-    {
-        if (slash != strSubnet.npos)
-        {
-            std::string strNetmask = strSubnet.substr(slash + 1);
-            uint8_t n;
-            if (ParseUInt8(strNetmask, &n)) {
-                // If valid number, assume CIDR variable-length subnet masking
-                ret = CSubNet(network, n);
-                return ret.IsValid();
-            }
-            else // If not a valid number, try full netmask syntax
-            {
-                CNetAddr netmask;
-                // Never allow lookup for netmask
-                if (LookupHost(strNetmask, netmask, false, dns_lookup_function)) {
-                    ret = CSubNet(network, netmask);
-                    return ret.IsValid();
+    const size_t slash_pos{subnet_str.find_last_of('/')};
+    const std::string str_addr{subnet_str.substr(0, slash_pos)};
+    CNetAddr addr;
+
+    if (LookupHost(str_addr, addr, /*fAllowLookup=*/false)) {
+        if (slash_pos != subnet_str.npos) {
+            const std::string netmask_str{subnet_str.substr(slash_pos + 1)};
+            uint8_t netmask;
+            if (ParseUInt8(netmask_str, &netmask)) {
+                // Valid number; assume CIDR variable-length subnet masking.
+                subnet_out = CSubNet{addr, netmask};
+                return subnet_out.IsValid();
+            } else {
+                // Invalid number; try full netmask syntax. Never allow lookup for netmask.
+                CNetAddr full_netmask;
+                if (LookupHost(netmask_str, full_netmask, /*fAllowLookup=*/false)) {
+                    subnet_out = CSubNet{addr, full_netmask};
+                    return subnet_out.IsValid();
                 }
             }
-        }
-        else // Single IP subnet (<ipv4>/32 or <ipv6>/128)
-        {
-            ret = CSubNet(network);
-            return ret.IsValid();
+        } else {
+            // Single IP subnet (<ipv4>/32 or <ipv6>/128).
+            subnet_out = CSubNet{addr};
+            return subnet_out.IsValid();
         }
     }
     return false;
diff --git a/src/netbase.h b/src/netbase.h
index 6a87c338a0..f3d8f15788 100644
--- a/src/netbase.h
+++ b/src/netbase.h
@@ -169,13 +169,14 @@ CService LookupNumeric(const std::string& name, uint16_t portDefault = 0, DNSLoo
  * Parse and resolve a specified subnet string into the appropriate internal
  * representation.
  *
- * @param strSubnet A string representation of a subnet of the form `network
- *                address [ "/", ( CIDR-style suffix | netmask ) ]`(e.g.
- *                `2001:db8::/32`, `192.0.2.0/255.255.255.0`, or `8.8.8.8`).
- *
- * @returns Whether the operation succeeded or not.
+ * @param[in]  subnet_str  A string representation of a subnet of the form
+ *                         `network address [ "/", ( CIDR-style suffix | netmask ) ]`
+ *                         e.g. "2001:db8::/32", "192.0.2.0/255.255.255.0" or "8.8.8.8".
+ * @param[out] subnet_out  Internal subnet representation, if parsable/resolvable
+ *                         from `subnet_str`.
+ * @returns whether the operation succeeded or not.
  */
-bool LookupSubNet(const std::string& strSubnet, CSubNet& subnet, DNSLookupFn dns_lookup_function = g_dns_lookup);
+bool LookupSubNet(const std::string& subnet_str, CSubNet& subnet_out);
 
 /**
  * Create a TCP socket in the given address family.
diff --git a/src/test/fuzz/netbase_dns_lookup.cpp b/src/test/fuzz/netbase_dns_lookup.cpp
index d01d413cff..31ea31744a 100644
--- a/src/test/fuzz/netbase_dns_lookup.cpp
+++ b/src/test/fuzz/netbase_dns_lookup.cpp
@@ -64,7 +64,7 @@ FUZZ_TARGET(netbase_dns_lookup)
     }
     {
         CSubNet resolved_subnet;
-        if (LookupSubNet(name, resolved_subnet, fuzzed_dns_lookup_function)) {
+        if (LookupSubNet(name, resolved_subnet)) {
             assert(resolved_subnet.IsValid());
         }
     }