[resolver] remove unused ctor for ServerAddress (#33148)

Co-authored-by: markdroth <[email protected]>
ClickHouse · May 16, 2023 · a78001a · a78001a
1 parent 63ecc4b
commit a78001a
Show file tree

Hide file tree

Showing 14 changed files with 56 additions and 73 deletions.
diff --git a/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc b/src/core/ext/filters/client_channel/resolver/dns/c_ares/grpc_ares_wrapper.cc
@@ -666,42 +666,41 @@ static void on_hostbyname_done_locked(void* arg, int status, int /*timeouts*/,
       if (hr->is_balancer) {
         args = args.Set(GRPC_ARG_DEFAULT_AUTHORITY, hr->host);
       }
+      grpc_resolved_address address;
+      memset(&address, 0, sizeof(address));
       switch (hostent->h_addrtype) {
         case AF_INET6: {
-          size_t addr_len = sizeof(struct sockaddr_in6);
-          struct sockaddr_in6 addr;
-          memset(&addr, 0, addr_len);
-          memcpy(&addr.sin6_addr, hostent->h_addr_list[i],
+          address.len = sizeof(struct sockaddr_in6);
+          auto* addr = reinterpret_cast<struct sockaddr_in6*>(&address.addr);
+          memcpy(&addr->sin6_addr, hostent->h_addr_list[i],
                  sizeof(struct in6_addr));
-          addr.sin6_family = static_cast<unsigned char>(hostent->h_addrtype);
-          addr.sin6_port = hr->port;
-          addresses.emplace_back(&addr, addr_len, args);
+          addr->sin6_family = static_cast<unsigned char>(hostent->h_addrtype);
+          addr->sin6_port = hr->port;
           char output[INET6_ADDRSTRLEN];
-          ares_inet_ntop(AF_INET6, &addr.sin6_addr, output, INET6_ADDRSTRLEN);
+          ares_inet_ntop(AF_INET6, &addr->sin6_addr, output, INET6_ADDRSTRLEN);
           GRPC_CARES_TRACE_LOG(
               "request:%p c-ares resolver gets a AF_INET6 result: \n"
               "  addr: %s\n  port: %d\n  sin6_scope_id: %d\n",
-              r, output, ntohs(hr->port), addr.sin6_scope_id);
+              r, output, ntohs(hr->port), addr->sin6_scope_id);
           break;
         }
         case AF_INET: {
-          size_t addr_len = sizeof(struct sockaddr_in);
-          struct sockaddr_in addr;
-          memset(&addr, 0, addr_len);
-          memcpy(&addr.sin_addr, hostent->h_addr_list[i],
+          address.len = sizeof(struct sockaddr_in);
+          auto* addr = reinterpret_cast<struct sockaddr_in*>(&address.addr);
+          memcpy(&addr->sin_addr, hostent->h_addr_list[i],
                  sizeof(struct in_addr));
-          addr.sin_family = static_cast<unsigned char>(hostent->h_addrtype);
-          addr.sin_port = hr->port;
-          addresses.emplace_back(&addr, addr_len, args);
+          addr->sin_family = static_cast<unsigned char>(hostent->h_addrtype);
+          addr->sin_port = hr->port;
           char output[INET_ADDRSTRLEN];
-          ares_inet_ntop(AF_INET, &addr.sin_addr, output, INET_ADDRSTRLEN);
+          ares_inet_ntop(AF_INET, &addr->sin_addr, output, INET_ADDRSTRLEN);
           GRPC_CARES_TRACE_LOG(
               "request:%p c-ares resolver gets a AF_INET result: \n"
               "  addr: %s\n  port: %d\n",
               r, output, ntohs(hr->port));
           break;
         }
       }
+      addresses.emplace_back(address, args);
     }
   } else {
     std::string error_msg = absl::StrFormat(
@@ -920,7 +919,7 @@ static bool inner_resolve_as_ip_literal_locked(
                                false /* log errors */)) {
     GPR_ASSERT(*addrs == nullptr);
     *addrs = std::make_unique<ServerAddressList>();
-    (*addrs)->emplace_back(addr.addr, addr.len, grpc_core::ChannelArgs());
+    (*addrs)->emplace_back(addr, grpc_core::ChannelArgs());
     return true;
   }
   return false;
@@ -979,23 +978,26 @@ static bool inner_maybe_resolve_localhost_manually_locked(
     GPR_ASSERT(*addrs == nullptr);
     *addrs = std::make_unique<grpc_core::ServerAddressList>();
     uint16_t numeric_port = grpc_strhtons(port->c_str());
+    grpc_resolved_address address;
     // Append the ipv6 loopback address.
-    struct sockaddr_in6 ipv6_loopback_addr;
-    memset(&ipv6_loopback_addr, 0, sizeof(ipv6_loopback_addr));
-    ((char*)&ipv6_loopback_addr.sin6_addr)[15] = 1;
-    ipv6_loopback_addr.sin6_family = AF_INET6;
-    ipv6_loopback_addr.sin6_port = numeric_port;
-    (*addrs)->emplace_back(&ipv6_loopback_addr, sizeof(ipv6_loopback_addr),
-                           grpc_core::ChannelArgs() /* args */);
+    memset(&address, 0, sizeof(address));
+    auto* ipv6_loopback_addr =
+        reinterpret_cast<struct sockaddr_in6*>(&address.addr);
+    ((char*)&ipv6_loopback_addr->sin6_addr)[15] = 1;
+    ipv6_loopback_addr->sin6_family = AF_INET6;
+    ipv6_loopback_addr->sin6_port = numeric_port;
+    address.len = sizeof(struct sockaddr_in6);
+    (*addrs)->emplace_back(address, grpc_core::ChannelArgs());
     // Append the ipv4 loopback address.
-    struct sockaddr_in ipv4_loopback_addr;
-    memset(&ipv4_loopback_addr, 0, sizeof(ipv4_loopback_addr));
-    ((char*)&ipv4_loopback_addr.sin_addr)[0] = 0x7f;
-    ((char*)&ipv4_loopback_addr.sin_addr)[3] = 0x01;
-    ipv4_loopback_addr.sin_family = AF_INET;
-    ipv4_loopback_addr.sin_port = numeric_port;
-    (*addrs)->emplace_back(&ipv4_loopback_addr, sizeof(ipv4_loopback_addr),
-                           grpc_core::ChannelArgs() /* args */);
+    memset(&address, 0, sizeof(address));
+    auto* ipv4_loopback_addr =
+        reinterpret_cast<struct sockaddr_in*>(&address.addr);
+    ((char*)&ipv4_loopback_addr->sin_addr)[0] = 0x7f;
+    ((char*)&ipv4_loopback_addr->sin_addr)[3] = 0x01;
+    ipv4_loopback_addr->sin_family = AF_INET;
+    ipv4_loopback_addr->sin_port = numeric_port;
+    address.len = sizeof(struct sockaddr_in);
+    (*addrs)->emplace_back(address, grpc_core::ChannelArgs());
     // Let the address sorter figure out which one should be tried first.
     grpc_cares_wrapper_address_sorting_sort(r, addrs->get());
     return true;

diff --git a/src/core/lib/resolver/server_address.cc b/src/core/lib/resolver/server_address.cc
@@ -57,14 +57,6 @@ ServerAddress::ServerAddress(
     std::map<const char*, std::unique_ptr<AttributeInterface>> attributes)
     : address_(address), args_(args), attributes_(std::move(attributes)) {}
 
-ServerAddress::ServerAddress(
-    const void* address, size_t address_len, const ChannelArgs& args,
-    std::map<const char*, std::unique_ptr<AttributeInterface>> attributes)
-    : args_(args), attributes_(std::move(attributes)) {
-  memcpy(address_.addr, address, address_len);
-  address_.len = static_cast<socklen_t>(address_len);
-}
-
 ServerAddress::ServerAddress(const ServerAddress& other)
     : address_(other.address_), args_(other.args_) {
   for (const auto& p : other.attributes_) {

diff --git a/src/core/lib/resolver/server_address.h b/src/core/lib/resolver/server_address.h
@@ -21,7 +21,6 @@
 
 #include <grpc/support/port_platform.h>
 
-#include <stddef.h>
 #include <stdint.h>
 
 #include <map>
@@ -65,14 +64,9 @@ class ServerAddress {
     virtual std::string ToString() const = 0;
   };
 
-  // Takes ownership of args.
   ServerAddress(const grpc_resolved_address& address, const ChannelArgs& args,
                 std::map<const char*, std::unique_ptr<AttributeInterface>>
                     attributes = {});
-  ServerAddress(const void* address, size_t address_len,
-                const ChannelArgs& args,
-                std::map<const char*, std::unique_ptr<AttributeInterface>>
-                    attributes = {});
 
   // Copyable.
   ServerAddress(const ServerAddress& other);

diff --git a/test/core/client_channel/resolvers/fake_resolver_test.cc b/test/core/client_channel/resolvers/fake_resolver_test.cc
@@ -113,7 +113,7 @@ static grpc_core::Resolver::Result create_new_resolver_result() {
     grpc_resolved_address address;
     EXPECT_TRUE(grpc_parse_uri(*uri, &address));
     absl::InlinedVector<grpc_arg, 2> args_to_add;
-    addresses.emplace_back(address.addr, address.len, grpc_core::ChannelArgs());
+    addresses.emplace_back(address, grpc_core::ChannelArgs());
   }
   ++test_counter;
   grpc_core::Resolver::Result result;

diff --git a/test/core/end2end/goaway_server_test.cc b/test/core/end2end/goaway_server_test.cc
@@ -185,12 +185,14 @@ static grpc_ares_request* my_dns_lookup_ares(
     error = GRPC_ERROR_CREATE("Forced Failure");
   } else {
     *addresses = std::make_unique<grpc_core::ServerAddressList>();
-    grpc_sockaddr_in sa;
-    memset(&sa, 0, sizeof(sa));
-    sa.sin_family = GRPC_AF_INET;
-    sa.sin_addr.s_addr = 0x100007f;
-    sa.sin_port = grpc_htons(static_cast<uint16_t>(g_resolve_port));
-    (*addresses)->emplace_back(&sa, sizeof(sa), grpc_core::ChannelArgs());
+    grpc_resolved_address address;
+    memset(&address, 0, sizeof(address));
+    auto* sa = reinterpret_cast<grpc_sockaddr_in*>(&address.addr);
+    sa->sin_family = GRPC_AF_INET;
+    sa->sin_addr.s_addr = 0x100007f;
+    sa->sin_port = grpc_htons(static_cast<uint16_t>(g_resolve_port));
+    address.len = sizeof(grpc_sockaddr_in);
+    (*addresses)->emplace_back(address, grpc_core::ChannelArgs());
     gpr_mu_unlock(&g_mu);
   }
   grpc_core::ExecCtx::Run(DEBUG_LOCATION, on_done, error);

diff --git a/test/core/iomgr/stranded_event_test.cc b/test/core/iomgr/stranded_event_test.cc
@@ -304,8 +304,7 @@ grpc_core::Resolver::Result BuildResolverResponse(
     }
     grpc_resolved_address address;
     GPR_ASSERT(grpc_parse_uri(*uri, &address));
-    result.addresses->emplace_back(address.addr, address.len,
-                                   grpc_core::ChannelArgs());
+    result.addresses->emplace_back(address, grpc_core::ChannelArgs());
   }
   return result;
 }

diff --git a/test/core/transport/chttp2/too_many_pings_test.cc b/test/core/transport/chttp2/too_many_pings_test.cc
@@ -443,8 +443,7 @@ grpc_core::Resolver::Result BuildResolverResult(
     }
     grpc_resolved_address address;
     GPR_ASSERT(grpc_parse_uri(*uri, &address));
-    result.addresses->emplace_back(address.addr, address.len,
-                                   grpc_core::ChannelArgs());
+    result.addresses->emplace_back(address, grpc_core::ChannelArgs());
   }
   return result;
 }

diff --git a/test/cpp/client/client_channel_stress_test.cc b/test/cpp/client/client_channel_stress_test.cc
@@ -231,9 +231,8 @@ class ClientChannelStressTest {
       grpc_resolved_address address;
       GPR_ASSERT(grpc_parse_uri(*lb_uri, &address));
       addresses.emplace_back(
-          address.addr, address.len,
-          grpc_core::ChannelArgs().Set(GRPC_ARG_DEFAULT_AUTHORITY,
-                                       addr.balancer_name));
+          address, grpc_core::ChannelArgs().Set(GRPC_ARG_DEFAULT_AUTHORITY,
+                                                addr.balancer_name));
     }
     return addresses;
   }

diff --git a/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc b/test/cpp/client/destroy_grpclb_channel_with_active_connect_stress_test.cc
@@ -68,7 +68,7 @@ void TryConnectAndDestroy() {
   grpc_resolved_address address;
   ASSERT_TRUE(grpc_parse_uri(*lb_uri, &address));
   grpc_core::ServerAddressList addresses;
-  addresses.emplace_back(address.addr, address.len, grpc_core::ChannelArgs());
+  addresses.emplace_back(address, grpc_core::ChannelArgs());
   grpc_core::Resolver::Result lb_address_result;
   lb_address_result.service_config = grpc_core::ServiceConfigImpl::Create(
       grpc_core::ChannelArgs(), "{\"loadBalancingConfig\":[{\"grpclb\":{}}]}");

diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc
@@ -264,8 +264,8 @@ class FakeResolverResponseGeneratorWrapper {
       if (attribute != nullptr) {
         attributes[attribute_key] = attribute->Copy();
       }
-      result.addresses->emplace_back(address.addr, address.len,
-                                     per_address_args, std::move(attributes));
+      result.addresses->emplace_back(address, per_address_args,
+                                     std::move(attributes));
     }
     if (result.addresses->empty()) {
       result.resolution_note = "fake resolver empty address list";

diff --git a/test/cpp/end2end/grpclb_end2end_test.cc b/test/cpp/end2end/grpclb_end2end_test.cc
@@ -566,9 +566,8 @@ class GrpclbEnd2endTest : public ::testing::Test {
       grpc_resolved_address address;
       GPR_ASSERT(grpc_parse_uri(*lb_uri, &address));
       addresses.emplace_back(
-          address.addr, address.len,
-          grpc_core::ChannelArgs().Set(GRPC_ARG_DEFAULT_AUTHORITY,
-                                       addr.balancer_name));
+          address, grpc_core::ChannelArgs().Set(GRPC_ARG_DEFAULT_AUTHORITY,
+                                                addr.balancer_name));
     }
     return addresses;
   }

diff --git a/test/cpp/end2end/service_config_end2end_test.cc b/test/cpp/end2end/service_config_end2end_test.cc
@@ -184,8 +184,7 @@ class ServiceConfigEnd2endTest : public ::testing::Test {
       GPR_ASSERT(lb_uri.ok());
       grpc_resolved_address address;
       GPR_ASSERT(grpc_parse_uri(*lb_uri, &address));
-      result.addresses->emplace_back(address.addr, address.len,
-                                     grpc_core::ChannelArgs());
+      result.addresses->emplace_back(address, grpc_core::ChannelArgs());
     }
     return result;
   }

diff --git a/test/cpp/end2end/xds/xds_cluster_type_end2end_test.cc b/test/cpp/end2end/xds/xds_cluster_type_end2end_test.cc
@@ -64,8 +64,7 @@ class ClusterTypeTest : public XdsEnd2endTest {
       GPR_ASSERT(lb_uri.ok());
       grpc_resolved_address address;
       GPR_ASSERT(grpc_parse_uri(*lb_uri, &address));
-      addresses.emplace_back(address.addr, address.len,
-                             grpc_core::ChannelArgs());
+      addresses.emplace_back(address, grpc_core::ChannelArgs());
     }
     return addresses;
   }

diff --git a/test/cpp/end2end/xds/xds_ring_hash_end2end_test.cc b/test/cpp/end2end/xds/xds_ring_hash_end2end_test.cc
@@ -71,8 +71,7 @@ class RingHashTest : public XdsEnd2endTest {
       GPR_ASSERT(lb_uri.ok());
       grpc_resolved_address address;
       GPR_ASSERT(grpc_parse_uri(*lb_uri, &address));
-      addresses.emplace_back(address.addr, address.len,
-                             grpc_core::ChannelArgs());
+      addresses.emplace_back(address, grpc_core::ChannelArgs());
     }
     return addresses;
   }