From a78001a087bb8dcc4fc114efe2e6ae0ea8a53ab7 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Tue, 16 May 2023 16:53:01 -0700 Subject: [PATCH] [resolver] remove unused ctor for ServerAddress (#33148) Co-authored-by: markdroth --- .../resolver/dns/c_ares/grpc_ares_wrapper.cc | 68 ++++++++++--------- src/core/lib/resolver/server_address.cc | 8 --- src/core/lib/resolver/server_address.h | 6 -- .../resolvers/fake_resolver_test.cc | 2 +- test/core/end2end/goaway_server_test.cc | 14 ++-- test/core/iomgr/stranded_event_test.cc | 3 +- .../transport/chttp2/too_many_pings_test.cc | 3 +- test/cpp/client/client_channel_stress_test.cc | 5 +- ...channel_with_active_connect_stress_test.cc | 2 +- test/cpp/end2end/client_lb_end2end_test.cc | 4 +- test/cpp/end2end/grpclb_end2end_test.cc | 5 +- .../end2end/service_config_end2end_test.cc | 3 +- .../xds/xds_cluster_type_end2end_test.cc | 3 +- .../end2end/xds/xds_ring_hash_end2end_test.cc | 3 +- 14 files changed, 56 insertions(+), 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 index 5f89193c128..cee19837b48 100644 --- 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,35 +666,33 @@ 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(&address.addr); + memcpy(&addr->sin6_addr, hostent->h_addr_list[i], sizeof(struct in6_addr)); - addr.sin6_family = static_cast(hostent->h_addrtype); - addr.sin6_port = hr->port; - addresses.emplace_back(&addr, addr_len, args); + addr->sin6_family = static_cast(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(&address.addr); + memcpy(&addr->sin_addr, hostent->h_addr_list[i], sizeof(struct in_addr)); - addr.sin_family = static_cast(hostent->h_addrtype); - addr.sin_port = hr->port; - addresses.emplace_back(&addr, addr_len, args); + addr->sin_family = static_cast(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", @@ -702,6 +700,7 @@ static void on_hostbyname_done_locked(void* arg, int status, int /*timeouts*/, 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(); - (*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(); 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(&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(&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 index 32da3165a51..e6a2748c866 100644 --- a/src/core/lib/resolver/server_address.cc +++ b/src/core/lib/resolver/server_address.cc @@ -57,14 +57,6 @@ ServerAddress::ServerAddress( std::map> attributes) : address_(address), args_(args), attributes_(std::move(attributes)) {} -ServerAddress::ServerAddress( - const void* address, size_t address_len, const ChannelArgs& args, - std::map> attributes) - : args_(args), attributes_(std::move(attributes)) { - memcpy(address_.addr, address, address_len); - address_.len = static_cast(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 index adf9c49db7c..05d8dedc4b6 100644 --- a/src/core/lib/resolver/server_address.h +++ b/src/core/lib/resolver/server_address.h @@ -21,7 +21,6 @@ #include -#include #include #include @@ -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> attributes = {}); - ServerAddress(const void* address, size_t address_len, - const ChannelArgs& args, - std::map> - 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 index 46b4dcffb77..3f9d50470b4 100644 --- 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 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 index 9b5c00d8277..a876b9a8710 100644 --- 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_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(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(&address.addr); + sa->sin_family = GRPC_AF_INET; + sa->sin_addr.s_addr = 0x100007f; + sa->sin_port = grpc_htons(static_cast(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 index f56a8524328..79ef0cc2a27 100644 --- 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 index 3da4bb1aa60..0d03aedf0d1 100644 --- 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 index 13b513e31ac..6485b06022a 100644 --- 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 index d096af45862..6382c97c99a 100644 --- 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 index e3b37856e86..31465b82b6c 100644 --- 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 index d326b2c09a0..de0c1e78428 100644 --- 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 index 8f6dde14b40..16604719cbf 100644 --- 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 index b67cfb756dd..3d16d797082 100644 --- 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 index b3defad5336..9a4b112e656 100644 --- 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; }