Remove old grpc_string_to_sockaddr and rename grpc_string_to_sockaddr…

…_new to grpc_string_to_sockaddr (grpc#25831)
smooth80 · Mar 29, 2021 · 5c49bf4 · 5c49bf4
1 parent 67886fb
commit 5c49bf4
Show file tree

Hide file tree

Showing 7 changed files with 44 additions and 46 deletions.
diff --git a/src/core/ext/xds/xds_api.cc b/src/core/ext/xds/xds_api.cc
@@ -2078,8 +2078,8 @@ grpc_error* CidrRangeParse(
     XdsApi::LdsUpdate::FilterChainMap::CidrRange* cidr_range) {
   std::string address_prefix = UpbStringToStdString(
       envoy_config_core_v3_CidrRange_address_prefix(cidr_range_proto));
-  grpc_error* error = grpc_string_to_sockaddr_new(&cidr_range->address,
-                                                  address_prefix.c_str(), 0);
+  grpc_error* error =
+      grpc_string_to_sockaddr(&cidr_range->address, address_prefix.c_str(), 0);
   if (error != GRPC_ERROR_NONE) return error;
   cidr_range->prefix_len = 0;
   auto* prefix_len_proto =
@@ -3008,8 +3008,7 @@ grpc_error* ServerAddressParseAndAppend(
   }
   // Populate grpc_resolved_address.
   grpc_resolved_address addr;
-  grpc_error* error =
-      grpc_string_to_sockaddr_new(&addr, address_str.c_str(), port);
+  grpc_error* error = grpc_string_to_sockaddr(&addr, address_str.c_str(), port);
   if (error != GRPC_ERROR_NONE) return error;
   // Append the address to the list.
   list->emplace_back(addr, nullptr);

diff --git a/src/core/ext/xds/xds_server_config_fetcher.cc b/src/core/ext/xds/xds_server_config_fetcher.cc
@@ -163,8 +163,14 @@ const XdsApi::LdsUpdate::FilterChainData* FindFilterChainDataForSourceType(
     return nullptr;
   }
   grpc_resolved_address source_addr;
-  grpc_string_to_sockaddr(&source_addr, host.c_str(),
-                          0 /* port doesn't matter here */);
+  grpc_error* error = grpc_string_to_sockaddr(&source_addr, host.c_str(),
+                                              0 /* port doesn't matter here */);
+  if (error != GRPC_ERROR_NONE) {
+    gpr_log(GPR_DEBUG, "Could not parse string to socket address: %s",
+            host.c_str());
+    GRPC_ERROR_UNREF(error);
+    return nullptr;
+  }
   // Use kAny only if kSameIporLoopback and kExternal are empty
   if (source_types_array[static_cast<int>(
                              XdsApi::LdsUpdate::FilterChainMap::
@@ -208,8 +214,14 @@ const XdsApi::LdsUpdate::FilterChainData* FindFilterChainDataForDestinationIp(
     return nullptr;
   }
   grpc_resolved_address destination_addr;
-  grpc_string_to_sockaddr(&destination_addr, host.c_str(),
-                          0 /* port doesn't matter here */);
+  grpc_error* error = grpc_string_to_sockaddr(&destination_addr, host.c_str(),
+                                              0 /* port doesn't matter here */);
+  if (error != GRPC_ERROR_NONE) {
+    gpr_log(GPR_DEBUG, "Could not parse string to socket address: %s",
+            host.c_str());
+    GRPC_ERROR_UNREF(error);
+    return nullptr;
+  }
   const XdsApi::LdsUpdate::FilterChainMap::DestinationIp* best_match = nullptr;
   for (const auto& entry : destination_ip_vector) {
     // Special case for catch-all

diff --git a/src/core/lib/channel/channelz.cc b/src/core/lib/channel/channelz.cc
@@ -437,14 +437,21 @@ void PopulateSocketAddressJson(Json::Object* json, const char* name,
       port_num = atoi(port.data());
     }
     grpc_resolved_address resolved_host;
-    grpc_string_to_sockaddr(&resolved_host, host.c_str(), port_num);
-    std::string packed_host = grpc_sockaddr_get_packed_host(&resolved_host);
-    std::string b64_host = absl::Base64Escape(packed_host);
-    data["tcpip_address"] = Json::Object{
-        {"port", port_num},
-        {"ip_address", b64_host},
-    };
-  } else if (uri.ok() && uri->scheme() == "unix") {
+    grpc_error* error =
+        grpc_string_to_sockaddr(&resolved_host, host.c_str(), port_num);
+    if (error == GRPC_ERROR_NONE) {
+      std::string packed_host = grpc_sockaddr_get_packed_host(&resolved_host);
+      std::string b64_host = absl::Base64Escape(packed_host);
+      data["tcpip_address"] = Json::Object{
+          {"port", port_num},
+          {"ip_address", b64_host},
+      };
+      (*json)[name] = std::move(data);
+      return;
+    }
+    GRPC_ERROR_UNREF(error);
+  }
+  if (uri.ok() && uri->scheme() == "unix") {
     data["uds_address"] = Json::Object{
         {"filename", uri->path()},
     };

diff --git a/src/core/lib/iomgr/sockaddr_utils.cc b/src/core/lib/iomgr/sockaddr_utils.cc
@@ -198,25 +198,8 @@ std::string grpc_sockaddr_to_string(const grpc_resolved_address* resolved_addr,
   return out;
 }
 
-void grpc_string_to_sockaddr(grpc_resolved_address* out, const char* addr,
-                             int port) {
-  memset(out, 0, sizeof(grpc_resolved_address));
-  grpc_sockaddr_in6* addr6 = reinterpret_cast<grpc_sockaddr_in6*>(out->addr);
-  grpc_sockaddr_in* addr4 = reinterpret_cast<grpc_sockaddr_in*>(out->addr);
-  if (grpc_inet_pton(GRPC_AF_INET6, addr, &addr6->sin6_addr) == 1) {
-    addr6->sin6_family = GRPC_AF_INET6;
-    out->len = sizeof(grpc_sockaddr_in6);
-  } else if (grpc_inet_pton(GRPC_AF_INET, addr, &addr4->sin_addr) == 1) {
-    addr4->sin_family = GRPC_AF_INET;
-    out->len = sizeof(grpc_sockaddr_in);
-  } else {
-    GPR_ASSERT(0);
-  }
-  grpc_sockaddr_set_port(out, port);
-}
-
-grpc_error* grpc_string_to_sockaddr_new(grpc_resolved_address* out,
-                                        const char* addr, int port) {
+grpc_error* grpc_string_to_sockaddr(grpc_resolved_address* out,
+                                    const char* addr, int port) {
   memset(out, 0, sizeof(grpc_resolved_address));
   grpc_sockaddr_in6* addr6 = reinterpret_cast<grpc_sockaddr_in6*>(out->addr);
   grpc_sockaddr_in* addr4 = reinterpret_cast<grpc_sockaddr_in*>(out->addr);

diff --git a/src/core/lib/iomgr/sockaddr_utils.h b/src/core/lib/iomgr/sockaddr_utils.h
@@ -64,17 +64,12 @@ int grpc_sockaddr_set_port(grpc_resolved_address* addr, int port);
 // If the normalize flag is enabled, ::ffff:0.0.0.0/96 IPv6 addresses are
 // displayed as plain IPv4.
 std::string grpc_sockaddr_to_string(const grpc_resolved_address* addr,
-                                    bool normalize);
-
-// TODO(yashykt): Remove this function and replace usages with
-// `grpc_string_to_sockaddr_new`
-void grpc_string_to_sockaddr(grpc_resolved_address* out, const char* addr,
-                             int port);
+                                    bool normalize) GRPC_MUST_USE_RESULT;
 
 // Newer form of grpc_string_to_sockaddr which returns an error instead of
 // crashing if \a addr is not IPv6/IPv6
-grpc_error* grpc_string_to_sockaddr_new(grpc_resolved_address* out,
-                                        const char* addr, int port);
+grpc_error* grpc_string_to_sockaddr(grpc_resolved_address* out,
+                                    const char* addr, int port);
 
 /* Returns the URI string corresponding to \a addr */
 std::string grpc_sockaddr_to_uri(const grpc_resolved_address* addr);

diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/iomgr.pxd.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/iomgr.pxd.pxi
@@ -123,7 +123,7 @@ cdef extern from "src/core/lib/iomgr/sockaddr_utils.h":
   int grpc_sockaddr_get_port(const grpc_resolved_address *addr);
   cppstring grpc_sockaddr_to_string(const grpc_resolved_address *addr,
                                     bool_t normalize);
-  void grpc_string_to_sockaddr(grpc_resolved_address *out, char* addr, int port);
+  grpc_error* grpc_string_to_sockaddr(grpc_resolved_address *out, char* addr, int port);
   int grpc_sockaddr_set_port(const grpc_resolved_address *resolved_addr,
                              int port)
   const char* grpc_sockaddr_get_uri_scheme(const grpc_resolved_address* resolved_addr)
diff --git a/test/core/iomgr/sockaddr_utils_test.cc b/test/core/iomgr/sockaddr_utils_test.cc
@@ -227,11 +227,13 @@ void VerifySocketAddressMatch(const std::string& ip_address,
                               const std::string& subnet, uint32_t mask_bits,
                               bool success) {
   grpc_resolved_address addr;
-  grpc_string_to_sockaddr(&addr, ip_address.c_str(), false);
+  ASSERT_EQ(grpc_string_to_sockaddr(&addr, ip_address.c_str(), false),
+            GRPC_ERROR_NONE);
   // Setting the port has no effect on the match.
   grpc_sockaddr_set_port(&addr, 12345);
   grpc_resolved_address subnet_addr;
-  grpc_string_to_sockaddr(&subnet_addr, subnet.c_str(), false);
+  ASSERT_EQ(grpc_string_to_sockaddr(&subnet_addr, subnet.c_str(), false),
+            GRPC_ERROR_NONE);
   grpc_sockaddr_mask_bits(&subnet_addr, mask_bits);
   EXPECT_EQ(grpc_sockaddr_match_subnet(&addr, &subnet_addr, mask_bits), success)
       << "IP=" << ip_address << " Subnet=" << subnet << " Mask=" << mask_bits;