Code review changes.

8 files changed, 24 insertions, 13 deletions

diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h
index 9dfcca9766..a228683d41 100644
--- a/include/grpc/grpc.h
+++ b/include/grpc/grpc.h
@@ -237,9 +237,12 @@ GRPCAPI struct census_context *grpc_census_call_get_context(grpc_call *call);
     created for. */
 GRPCAPI char *grpc_channel_get_target(grpc_channel *channel);
 
-/** Request info about the channel. */
+/** Request info about the channel.
+    \a channel_info indicates what information is being requested and
+    how that information will be returned.
+    \a channel_info is owned by the caller. */
 GRPCAPI void grpc_channel_get_info(grpc_channel *channel,
-                                   grpc_channel_info *channel_info);
+                                   const grpc_channel_info *channel_info);
 
 /** Create a client channel to 'target'. Additional channel level configuration
     MAY be provided by grpc_channel_args, though the expectation is that most
diff --git a/src/core/ext/client_channel/client_channel.c b/src/core/ext/client_channel/client_channel.c
index 3b89f938c2..1bed965ed1 100644
--- a/src/core/ext/client_channel/client_channel.c
+++ b/src/core/ext/client_channel/client_channel.c
@@ -243,7 +243,7 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg,
         grpc_channel_args_find(lb_policy_args.args, GRPC_ARG_LB_POLICY_NAME);
     if (channel_arg != NULL) {
       GPR_ASSERT(channel_arg->type == GRPC_ARG_STRING);
-      lb_policy_name = gpr_strdup(channel_arg->value.string);
+      lb_policy_name = channel_arg->value.string;
     }
     // Special case: If all of the addresses are balancer addresses,
     // assume that we should use the grpclb policy, regardless of what the
@@ -267,14 +267,13 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg,
                   "addresses, no backend addresses -- forcing use of grpclb LB "
                   "policy",
                   lb_policy_name);
-          gpr_free(lb_policy_name);
         }
-        lb_policy_name = gpr_strdup("grpclb");
+        lb_policy_name = "grpclb";
       }
     }
     // Use pick_first if nothing was specified and we didn't select grpclb
     // above.
-    if (lb_policy_name == NULL) lb_policy_name = gpr_strdup("pick_first");
+    if (lb_policy_name == NULL) lb_policy_name = "pick_first";
 
     lb_policy =
         grpc_lb_policy_create(exec_ctx, lb_policy_name, &lb_policy_args);
@@ -292,6 +291,10 @@ static void on_resolver_result_changed(grpc_exec_ctx *exec_ctx, void *arg,
           (grpc_method_config_table *)channel_arg->value.pointer.p,
           method_config_convert_value, &method_parameters_vtable);
     }
+    // Before we clean up, save a copy of lb_policy_name, since it might
+    // be pointing to data inside chand->resolver_result.
+    // The copy will be saved in chand->lb_policy_name below.
+    lb_policy_name = gpr_strdup(lb_policy_name);
     grpc_channel_args_destroy(chand->resolver_result);
     chand->resolver_result = NULL;
   }
@@ -435,7 +438,7 @@ static void cc_start_transport_op(grpc_exec_ctx *exec_ctx,
 
 static void cc_get_channel_info(grpc_exec_ctx *exec_ctx,
                                 grpc_channel_element *elem,
-                                grpc_channel_info *info) {
+                                const grpc_channel_info *info) {
   channel_data *chand = elem->channel_data;
   gpr_mu_lock(&chand->mu);
   if (info->lb_policy_name != NULL) {
diff --git a/src/core/lib/channel/channel_stack.c b/src/core/lib/channel/channel_stack.c
index 3370f0ef45..c6c90d8c16 100644
--- a/src/core/lib/channel/channel_stack.c
+++ b/src/core/lib/channel/channel_stack.c
@@ -257,7 +257,7 @@ char *grpc_call_next_get_peer(grpc_exec_ctx *exec_ctx,
 
 void grpc_channel_next_get_info(grpc_exec_ctx *exec_ctx,
                                 grpc_channel_element *elem,
-                                grpc_channel_info *channel_info) {
+                                const grpc_channel_info *channel_info) {
   grpc_channel_element *next_elem = elem + 1;
   return next_elem->filter->get_channel_info(exec_ctx, next_elem, channel_info);
 }
diff --git a/src/core/lib/channel/channel_stack.h b/src/core/lib/channel/channel_stack.h
index 6fbcd87064..9b9ded0aee 100644
--- a/src/core/lib/channel/channel_stack.h
+++ b/src/core/lib/channel/channel_stack.h
@@ -158,7 +158,7 @@ typedef struct {
 
   /* Implement grpc_channel_get_info() */
   void (*get_channel_info)(grpc_exec_ctx *exec_ctx, grpc_channel_element *elem,
-                           grpc_channel_info *channel_info);
+                           const grpc_channel_info *channel_info);
 
   /* The name of this filter */
   const char *name;
@@ -280,7 +280,7 @@ char *grpc_call_next_get_peer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem);
 /* Pass through a request to get_channel_info() to the next child element */
 void grpc_channel_next_get_info(grpc_exec_ctx *exec_ctx,
                                 grpc_channel_element *elem,
-                                grpc_channel_info *channel_info);
+                                const grpc_channel_info *channel_info);
 
 /* Given the top element of a channel stack, get the channel stack itself */
 grpc_channel_stack *grpc_channel_stack_from_top_element(
diff --git a/src/core/lib/channel/connected_channel.c b/src/core/lib/channel/connected_channel.c
index 6ae2b3b6b7..bb6986438e 100644
--- a/src/core/lib/channel/connected_channel.c
+++ b/src/core/lib/channel/connected_channel.c
@@ -137,7 +137,7 @@ static char *con_get_peer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem) {
 /* No-op. */
 static void con_get_channel_info(grpc_exec_ctx *exec_ctx,
                                  grpc_channel_element *elem,
-                                 grpc_channel_info *channel_info) {}
+                                 const grpc_channel_info *channel_info) {}
 
 static const grpc_channel_filter connected_channel_filter = {
     con_start_transport_stream_op,
diff --git a/src/core/lib/surface/channel.c b/src/core/lib/surface/channel.c
index 22bf85b126..1389df6886 100644
--- a/src/core/lib/surface/channel.c
+++ b/src/core/lib/surface/channel.c
@@ -176,7 +176,7 @@ char *grpc_channel_get_target(grpc_channel *channel) {
 }
 
 void grpc_channel_get_info(grpc_channel *channel,
-                           grpc_channel_info *channel_info) {
+                           const grpc_channel_info *channel_info) {
   grpc_exec_ctx exec_ctx = GRPC_EXEC_CTX_INIT;
   grpc_channel_element *elem =
       grpc_channel_stack_element(CHANNEL_STACK_FROM_CHANNEL(channel), 0);
diff --git a/src/core/lib/surface/lame_client.c b/src/core/lib/surface/lame_client.c
index 6fd1dd921c..d0df8e7e17 100644
--- a/src/core/lib/surface/lame_client.c
+++ b/src/core/lib/surface/lame_client.c
@@ -90,7 +90,7 @@ static char *lame_get_peer(grpc_exec_ctx *exec_ctx, grpc_call_element *elem) {
 
 static void lame_get_channel_info(grpc_exec_ctx *exec_ctx,
                                   grpc_channel_element *elem,
-                                  grpc_channel_info *channel_info) {}
+                                  const grpc_channel_info *channel_info) {}
 
 static void lame_start_transport_op(grpc_exec_ctx *exec_ctx,
                                     grpc_channel_element *elem,
diff --git a/test/core/client_channel/lb_policies_test.c b/test/core/client_channel/lb_policies_test.c
index 857ba6bd52..7c639cbeb9 100644
--- a/test/core/client_channel/lb_policies_test.c
+++ b/test/core/client_channel/lb_policies_test.c
@@ -643,6 +643,7 @@ static void test_get_channel_info() {
       "test:127.0.0.1:1234?lb_policy=round_robin", NULL, NULL);
   // Ensures that resolver returns.
   grpc_channel_check_connectivity_state(channel, true /* try_to_connect */);
+  // Use grpc_channel_get_info() to get LB policy name.
   char *lb_policy_name = NULL;
   grpc_channel_info channel_info;
   channel_info.lb_policy_name = &lb_policy_name;
@@ -650,6 +651,10 @@ static void test_get_channel_info() {
   GPR_ASSERT(lb_policy_name != NULL);
   GPR_ASSERT(strcmp(lb_policy_name, "round_robin") == 0);
   gpr_free(lb_policy_name);
+  // Try again without requesting anything.  This is a no-op.
+  channel_info.lb_policy_name = NULL;
+  grpc_channel_get_info(channel, &channel_info);
+  // Clean up.
   grpc_channel_destroy(channel);
 }