Improvements to Fake Resolver

8 files changed, 101 insertions, 37 deletions

diff --git a/src/core/ext/filters/client_channel/lb_policy_factory.c b/src/core/ext/filters/client_channel/lb_policy_factory.c
index e2af216b89..89b8bf8951 100644
--- a/src/core/ext/filters/client_channel/lb_policy_factory.c
+++ b/src/core/ext/filters/client_channel/lb_policy_factory.c
@@ -36,16 +36,18 @@
 #include <grpc/support/alloc.h>
 #include <grpc/support/string_util.h>
 
+#include "src/core/lib/channel/channel_args.h"
+
 #include "src/core/ext/filters/client_channel/lb_policy_factory.h"
+#include "src/core/ext/filters/client_channel/parse_address.h"
 
 grpc_lb_addresses* grpc_lb_addresses_create(
     size_t num_addresses, const grpc_lb_user_data_vtable* user_data_vtable) {
-  grpc_lb_addresses* addresses = gpr_malloc(sizeof(grpc_lb_addresses));
+  grpc_lb_addresses* addresses = gpr_zalloc(sizeof(grpc_lb_addresses));
   addresses->num_addresses = num_addresses;
   addresses->user_data_vtable = user_data_vtable;
   const size_t addresses_size = sizeof(grpc_lb_address) * num_addresses;
-  addresses->addresses = gpr_malloc(addresses_size);
-  memset(addresses->addresses, 0, addresses_size);
+  addresses->addresses = gpr_zalloc(addresses_size);
   return addresses;
 }
 
@@ -69,7 +71,7 @@ grpc_lb_addresses* grpc_lb_addresses_copy(const grpc_lb_addresses* addresses) {
 
 void grpc_lb_addresses_set_address(grpc_lb_addresses* addresses, size_t index,
                                    void* address, size_t address_len,
-                                   bool is_balancer, char* balancer_name,
+                                   bool is_balancer, const char* balancer_name,
                                    void* user_data) {
   GPR_ASSERT(index < addresses->num_addresses);
   if (user_data != NULL) GPR_ASSERT(addresses->user_data_vtable != NULL);
@@ -77,10 +79,22 @@ void grpc_lb_addresses_set_address(grpc_lb_addresses* addresses, size_t index,
   memcpy(target->address.addr, address, address_len);
   target->address.len = address_len;
   target->is_balancer = is_balancer;
-  target->balancer_name = balancer_name;
+  target->balancer_name = gpr_strdup(balancer_name);
   target->user_data = user_data;
 }
 
+bool grpc_lb_addresses_set_address_from_uri(grpc_lb_addresses* addresses,
+                                            size_t index, const grpc_uri* uri,
+                                            bool is_balancer,
+                                            const char* balancer_name,
+                                            void* user_data) {
+  grpc_resolved_address address;
+  if (!grpc_parse_uri(uri, &address)) return false;
+  grpc_lb_addresses_set_address(addresses, index, address.addr, address.len,
+                                is_balancer, balancer_name, user_data);
+  return true;
+}
+
 int grpc_lb_addresses_cmp(const grpc_lb_addresses* addresses1,
                           const grpc_lb_addresses* addresses2) {
   if (addresses1->num_addresses > addresses2->num_addresses) return 1;
@@ -147,6 +161,15 @@ grpc_arg grpc_lb_addresses_create_channel_arg(
   return arg;
 }
 
+grpc_lb_addresses* grpc_lb_addresses_find_channel_arg(
+    const grpc_channel_args* channel_args) {
+  const grpc_arg* lb_addresses_arg =
+      grpc_channel_args_find(channel_args, GRPC_ARG_LB_ADDRESSES);
+  if (lb_addresses_arg == NULL || lb_addresses_arg->type != GRPC_ARG_POINTER)
+    return NULL;
+  return lb_addresses_arg->value.pointer.p;
+}
+
 void grpc_lb_policy_factory_ref(grpc_lb_policy_factory* factory) {
   factory->vtable->ref(factory);
 }
diff --git a/src/core/ext/filters/client_channel/lb_policy_factory.h b/src/core/ext/filters/client_channel/lb_policy_factory.h
index 81ab12ec8f..9d6c0fc139 100644
--- a/src/core/ext/filters/client_channel/lb_policy_factory.h
+++ b/src/core/ext/filters/client_channel/lb_policy_factory.h
@@ -34,12 +34,13 @@
 #ifndef GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_FACTORY_H
 #define GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_LB_POLICY_FACTORY_H
 
-#include "src/core/ext/filters/client_channel/client_channel_factory.h"
-#include "src/core/ext/filters/client_channel/lb_policy.h"
-
 #include "src/core/lib/iomgr/exec_ctx.h"
 #include "src/core/lib/iomgr/resolve_address.h"
 
+#include "src/core/ext/filters/client_channel/client_channel_factory.h"
+#include "src/core/ext/filters/client_channel/lb_policy.h"
+#include "src/core/ext/filters/client_channel/uri_parser.h"
+
 // Channel arg key for grpc_lb_addresses.
 #define GRPC_ARG_LB_ADDRESSES "grpc.lb_addresses"
 
@@ -88,9 +89,18 @@ grpc_lb_addresses *grpc_lb_addresses_copy(const grpc_lb_addresses *addresses);
  * Takes ownership of \a balancer_name. */
 void grpc_lb_addresses_set_address(grpc_lb_addresses *addresses, size_t index,
                                    void *address, size_t address_len,
-                                   bool is_balancer, char *balancer_name,
+                                   bool is_balancer, const char *balancer_name,
                                    void *user_data);
 
+/** Sets the value of the address at index \a index of \a addresses from \a uri.
+ * Returns true upon success, false otherwise. Takes ownership of \a
+ * balancer_name. */
+bool grpc_lb_addresses_set_address_from_uri(grpc_lb_addresses *addresses,
+                                            size_t index, const grpc_uri *uri,
+                                            bool is_balancer,
+                                            const char *balancer_name,
+                                            void *user_data);
+
 /** Compares \a addresses1 and \a addresses2. */
 int grpc_lb_addresses_cmp(const grpc_lb_addresses *addresses1,
                           const grpc_lb_addresses *addresses2);
@@ -103,6 +113,10 @@ void grpc_lb_addresses_destroy(grpc_exec_ctx *exec_ctx,
 grpc_arg grpc_lb_addresses_create_channel_arg(
     const grpc_lb_addresses *addresses);
 
+/** Returns the \a grpc_lb_addresses instance in \a channel_args or NULL */
+grpc_lb_addresses *grpc_lb_addresses_find_channel_arg(
+    const grpc_channel_args *channel_args);
+
 /** Arguments passed to LB policies. */
 typedef struct grpc_lb_policy_args {
   grpc_client_channel_factory *client_channel_factory;
diff --git a/src/core/ext/filters/client_channel/parse_address.c b/src/core/ext/filters/client_channel/parse_address.c
index 0c97062075..edc6ce697d 100644
--- a/src/core/ext/filters/client_channel/parse_address.c
+++ b/src/core/ext/filters/client_channel/parse_address.c
@@ -48,7 +48,12 @@
 
 #ifdef GRPC_HAVE_UNIX_SOCKET
 
-int parse_unix(grpc_uri *uri, grpc_resolved_address *resolved_addr) {
+bool grpc_parse_unix(const grpc_uri *uri,
+                     grpc_resolved_address *resolved_addr) {
+  if (strcmp("unix", uri->scheme) != 0) {
+    gpr_log(GPR_ERROR, "Expected 'unix' scheme, got '%s'", uri->scheme);
+    return false;
+  }
   struct sockaddr_un *un = (struct sockaddr_un *)resolved_addr->addr;
   const size_t maxlen = sizeof(un->sun_path);
   const size_t path_len = strnlen(uri->path, maxlen);
@@ -61,21 +66,29 @@ int parse_unix(grpc_uri *uri, grpc_resolved_address *resolved_addr) {
 
 #else /* GRPC_HAVE_UNIX_SOCKET */
 
-int parse_unix(grpc_uri *uri, grpc_resolved_address *resolved_addr) { abort(); }
+bool grpc_parse_unix(const grpc_uri *uri,
+                     grpc_resolved_address *resolved_addr) {
+  abort();
+}
 
 #endif /* GRPC_HAVE_UNIX_SOCKET */
 
-int parse_ipv4(grpc_uri *uri, grpc_resolved_address *resolved_addr) {
+bool grpc_parse_ipv4(const grpc_uri *uri,
+                     grpc_resolved_address *resolved_addr) {
+  if (strcmp("ipv4", uri->scheme) != 0) {
+    gpr_log(GPR_ERROR, "Expected 'ipv4' scheme, got '%s'", uri->scheme);
+    return false;
+  }
   const char *host_port = uri->path;
   char *host;
   char *port;
   int port_num;
-  int result = 0;
+  bool result = false;
   struct sockaddr_in *in = (struct sockaddr_in *)resolved_addr->addr;
 
   if (*host_port == '/') ++host_port;
   if (!gpr_split_host_port(host_port, &host, &port)) {
-    return 0;
+    return false;
   }
 
   memset(resolved_addr, 0, sizeof(grpc_resolved_address));
@@ -98,14 +111,19 @@ int parse_ipv4(grpc_uri *uri, grpc_resolved_address *resolved_addr) {
     goto done;
   }
 
-  result = 1;
+  result = true;
 done:
   gpr_free(host);
   gpr_free(port);
   return result;
 }
 
-int parse_ipv6(grpc_uri *uri, grpc_resolved_address *resolved_addr) {
+bool grpc_parse_ipv6(const grpc_uri *uri,
+                     grpc_resolved_address *resolved_addr) {
+  if (strcmp("ipv6", uri->scheme) != 0) {
+    gpr_log(GPR_ERROR, "Expected 'ipv6' scheme, got '%s'", uri->scheme);
+    return false;
+  }
   const char *host_port = uri->path;
   char *host;
   char *port;
@@ -168,3 +186,15 @@ done:
   gpr_free(port);
   return result;
 }
+
+bool grpc_parse_uri(const grpc_uri *uri, grpc_resolved_address *resolved_addr) {
+  if (strcmp("unix", uri->scheme) == 0) {
+    return grpc_parse_unix(uri, resolved_addr);
+  } else if (strcmp("ipv4", uri->scheme) == 0) {
+    return grpc_parse_ipv4(uri, resolved_addr);
+  } else if (strcmp("ipv6", uri->scheme) == 0) {
+    return grpc_parse_ipv6(uri, resolved_addr);
+  }
+  gpr_log(GPR_ERROR, "Can't parse scheme '%s'", uri->scheme);
+  return false;
+}
diff --git a/src/core/ext/filters/client_channel/parse_address.h b/src/core/ext/filters/client_channel/parse_address.h
index c8d77baa00..fa7ea33a00 100644
--- a/src/core/ext/filters/client_channel/parse_address.h
+++ b/src/core/ext/filters/client_channel/parse_address.h
@@ -39,16 +39,19 @@
 #include "src/core/ext/filters/client_channel/uri_parser.h"
 #include "src/core/lib/iomgr/resolve_address.h"
 
-/** Populate \a addr and \a len from \a uri, whose path is expected to contain a
+/** Populate \a resolved_addr from \a uri, whose path is expected to contain a
  * unix socket path. Returns true upon success. */
-int parse_unix(grpc_uri *uri, grpc_resolved_address *resolved_addr);
+bool grpc_parse_unix(const grpc_uri *uri, grpc_resolved_address *resolved_addr);
 
-/** Populate /a addr and \a len from \a uri, whose path is expected to contain a
- * host:port pair. Returns true upon success. */
-int parse_ipv4(grpc_uri *uri, grpc_resolved_address *resolved_addr);
+/** Populate \a resolved_addr from \a uri, whose path is expected to contain an
+ * IPv4 host:port pair. Returns true upon success. */
+bool grpc_parse_ipv4(const grpc_uri *uri, grpc_resolved_address *resolved_addr);
 
-/** Populate /a addr and \a len from \a uri, whose path is expected to contain a
- * host:port pair. Returns true upon success. */
-int parse_ipv6(grpc_uri *uri, grpc_resolved_address *resolved_addr);
+/** Populate \a resolved_addr from \a uri, whose path is expected to contain an
+ * IPv6 host:port pair. Returns true upon success. */
+bool grpc_parse_ipv6(const grpc_uri *uri, grpc_resolved_address *resolved_addr);
+
+/** Populate \a resolved_addr from \a uri. Returns true upon success. */
+bool grpc_parse_uri(const grpc_uri *uri, grpc_resolved_address *resolved_addr);
 
 #endif /* GRPC_CORE_EXT_FILTERS_CLIENT_CHANNEL_PARSE_ADDRESS_H */
diff --git a/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.c b/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.c
index 54f020d691..4d7d878c23 100644
--- a/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.c
+++ b/src/core/ext/filters/client_channel/resolver/sockaddr/sockaddr_resolver.c
@@ -157,8 +157,8 @@ static void do_nothing(void *ignored) {}
 
 static grpc_resolver *sockaddr_create(grpc_exec_ctx *exec_ctx,
                                       grpc_resolver_args *args,
-                                      int parse(grpc_uri *uri,
-                                                grpc_resolved_address *dst)) {
+                                      bool parse(const grpc_uri *uri,
+                                                 grpc_resolved_address *dst)) {
   if (0 != strcmp(args->uri->authority, "")) {
     gpr_log(GPR_ERROR, "authority based uri's not supported by the %s scheme",
             args->uri->scheme);
@@ -209,7 +209,7 @@ static void sockaddr_factory_unref(grpc_resolver_factory *factory) {}
   static grpc_resolver *name##_factory_create_resolver(                     \
       grpc_exec_ctx *exec_ctx, grpc_resolver_factory *factory,              \
       grpc_resolver_args *args) {                                           \
-    return sockaddr_create(exec_ctx, args, parse_##name);                   \
+    return sockaddr_create(exec_ctx, args, grpc_parse_##name);              \
   }                                                                         \
   static const grpc_resolver_factory_vtable name##_factory_vtable = {       \
       sockaddr_factory_ref, sockaddr_factory_unref,                         \
diff --git a/src/core/ext/filters/client_channel/subchannel.c b/src/core/ext/filters/client_channel/subchannel.c
index 9a7a7a0ee5..362b4a4a28 100644
--- a/src/core/ext/filters/client_channel/subchannel.c
+++ b/src/core/ext/filters/client_channel/subchannel.c
@@ -797,13 +797,7 @@ static void grpc_uri_to_sockaddr(grpc_exec_ctx *exec_ctx, const char *uri_str,
                                  grpc_resolved_address *addr) {
   grpc_uri *uri = grpc_uri_parse(exec_ctx, uri_str, 0 /* suppress_errors */);
   GPR_ASSERT(uri != NULL);
-  if (strcmp(uri->scheme, "ipv4") == 0) {
-    GPR_ASSERT(parse_ipv4(uri, addr));
-  } else if (strcmp(uri->scheme, "ipv6") == 0) {
-    GPR_ASSERT(parse_ipv6(uri, addr));
-  } else {
-    GPR_ASSERT(parse_unix(uri, addr));
-  }
+  if (!grpc_parse_uri(uri, addr)) memset(addr, 0, sizeof(*addr));
   grpc_uri_destroy(uri);
 }
 
diff --git a/src/core/ext/filters/client_channel/uri_parser.c b/src/core/ext/filters/client_channel/uri_parser.c
index f28db59e27..b233d835cb 100644
--- a/src/core/ext/filters/client_channel/uri_parser.c
+++ b/src/core/ext/filters/client_channel/uri_parser.c
@@ -50,7 +50,7 @@
 #define NOT_SET (~(size_t)0)
 
 static grpc_uri *bad_uri(const char *uri_text, size_t pos, const char *section,
-                         int suppress_errors) {
+                         bool suppress_errors) {
   char *line_prefix;
   size_t pfx_len;
 
@@ -197,7 +197,7 @@ static void parse_query_parts(grpc_uri *uri) {
 }
 
 grpc_uri *grpc_uri_parse(grpc_exec_ctx *exec_ctx, const char *uri_text,
-                         int suppress_errors) {
+                         bool suppress_errors) {
   grpc_uri *uri;
   size_t scheme_begin = 0;
   size_t scheme_end = NOT_SET;
diff --git a/src/core/ext/filters/client_channel/uri_parser.h b/src/core/ext/filters/client_channel/uri_parser.h
index 2698d448d8..b889040b16 100644
--- a/src/core/ext/filters/client_channel/uri_parser.h
+++ b/src/core/ext/filters/client_channel/uri_parser.h
@@ -53,7 +53,7 @@ typedef struct {
 
 /** parse a uri, return NULL on failure */
 grpc_uri *grpc_uri_parse(grpc_exec_ctx *exec_ctx, const char *uri_text,
-                         int suppress_errors);
+                         bool suppress_errors);
 
 /** return the part of a query string after the '=' in "?key=xxx&...", or NULL
  * if key is not present */