Start resolving memory issues in C++ metadata

include/grpc++/impl/codegen/call.h

@@ -63,19 +63,6 @@ class CallHook;
 class CompletionQueue;
 extern CoreCodegenInterface* g_core_codegen_interface;
 
-inline void FillMetadataMap(
-    grpc_metadata_array* arr,
-    std::multimap<grpc::string_ref, grpc::string_ref>* metadata) {
-  for (size_t i = 0; i < arr->count; i++) {
-    // TODO(yangg) handle duplicates?
-    metadata->insert(std::pair<grpc::string_ref, grpc::string_ref>(
-        StringRefFromSlice(arr->metadata[i].key),
-        StringRefFromSlice(arr->metadata[i].value)));
-  }
-  g_core_codegen_interface->grpc_metadata_array_destroy(arr);
-  g_core_codegen_interface->grpc_metadata_array_init(arr);
-}
-
 // TODO(yangg) if the map is changed before we send, the pointers will be a
 // mess. Make sure it does not happen.
 inline grpc_metadata* FillMetadataArray(
@@ -474,32 +461,30 @@ class CallOpServerSendStatus {
 
 class CallOpRecvInitialMetadata {
  public:
-  CallOpRecvInitialMetadata() : recv_initial_metadata_(nullptr) {}
+  CallOpRecvInitialMetadata() : metadata_map_(nullptr) {}
 
   void RecvInitialMetadata(ClientContext* context) {
     context->initial_metadata_received_ = true;
-    recv_initial_metadata_ = &context->recv_initial_metadata_;
+    metadata_map_ = &context->recv_initial_metadata_;
   }
 
  protected:
   void AddOp(grpc_op* ops, size_t* nops) {
-    if (!recv_initial_metadata_) return;
+    if (metadata_map_ == nullptr) return;
-    memset(&recv_initial_metadata_arr_, 0, sizeof(recv_initial_metadata_arr_));
     grpc_op* op = &ops[(*nops)++];
     op->op = GRPC_OP_RECV_INITIAL_METADATA;
-    op->data.recv_initial_metadata = &recv_initial_metadata_arr_;
+    op->data.recv_initial_metadata = metadata_map_->arr();
     op->flags = 0;
     op->reserved = NULL;
   }
   void FinishOp(bool* status, int max_receive_message_size) {
-    if (recv_initial_metadata_ == nullptr) return;
+    if (metadata_map_ == nullptr) return;
-    FillMetadataMap(&recv_initial_metadata_arr_, recv_initial_metadata_);
+    metadata_map_->FillMap();
-    recv_initial_metadata_ = nullptr;
+    metadata_map_ = nullptr;
   }
 
  private:
-  std::multimap<grpc::string_ref, grpc::string_ref>* recv_initial_metadata_;
+  MetadataMap* metadata_map_;
-  grpc_metadata_array recv_initial_metadata_arr_;
 };
 
 class CallOpClientRecvStatus {
@@ -507,19 +492,16 @@ class CallOpClientRecvStatus {
   CallOpClientRecvStatus() : recv_status_(nullptr) {}
 
   void ClientRecvStatus(ClientContext* context, Status* status) {
-    recv_trailing_metadata_ = &context->trailing_metadata_;
+    metadata_map_ = &context->trailing_metadata_;
     recv_status_ = status;
   }
 
  protected:
   void AddOp(grpc_op* ops, size_t* nops) {
     if (recv_status_ == nullptr) return;
-    memset(&recv_trailing_metadata_arr_, 0,
-           sizeof(recv_trailing_metadata_arr_));
     grpc_op* op = &ops[(*nops)++];
     op->op = GRPC_OP_RECV_STATUS_ON_CLIENT;
-    op->data.recv_status_on_client.trailing_metadata =
+    op->data.recv_status_on_client.trailing_metadata = metadata_map_->arr();
-        &recv_trailing_metadata_arr_;
     op->data.recv_status_on_client.status = &status_code_;
     op->data.recv_status_on_client.status_details = &status_details_;
     op->flags = 0;
@@ -528,7 +510,7 @@ class CallOpClientRecvStatus {
 
   void FinishOp(bool* status, int max_receive_message_size) {
     if (recv_status_ == nullptr) return;
-    FillMetadataMap(&recv_trailing_metadata_arr_, recv_trailing_metadata_);
+    metadata_map_->FillMap();
     *recv_status_ = Status(static_cast<StatusCode>(status_code_),
                            grpc::string(GRPC_SLICE_START_PTR(status_details_),
                                         GRPC_SLICE_END_PTR(status_details_)));
@@ -537,9 +519,8 @@ class CallOpClientRecvStatus {
   }
 
  private:
-  std::multimap<grpc::string_ref, grpc::string_ref>* recv_trailing_metadata_;
+  MetadataMap* metadata_map_;
   Status* recv_status_;
-  grpc_metadata_array recv_trailing_metadata_arr_;
   grpc_status_code status_code_;
   grpc_slice status_details_;
 };

include/grpc++/impl/codegen/client_context.h

@@ -57,7 +57,9 @@
 #include <grpc++/impl/codegen/config.h>
 #include <grpc++/impl/codegen/core_codegen_interface.h>
 #include <grpc++/impl/codegen/create_auth_context.h>
+#include <grpc++/impl/codegen/metadata_map.h>
 #include <grpc++/impl/codegen/security/auth_context.h>
+#include <grpc++/impl/codegen/slice.h>
 #include <grpc++/impl/codegen/status.h>
 #include <grpc++/impl/codegen/string_ref.h>
 #include <grpc++/impl/codegen/time.h>
@@ -193,7 +195,7 @@ class ClientContext {
   const std::multimap<grpc::string_ref, grpc::string_ref>&
   GetServerInitialMetadata() const {
     GPR_CODEGEN_ASSERT(initial_metadata_received_);
-    return recv_initial_metadata_;
+    return *recv_initial_metadata_.map();
   }
 
   /// Return a collection of trailing metadata key-value pairs. Note that keys
@@ -205,7 +207,7 @@ class ClientContext {
   const std::multimap<grpc::string_ref, grpc::string_ref>&
   GetServerTrailingMetadata() const {
     // TODO(yangg) check finished
-    return trailing_metadata_;
+    return *trailing_metadata_.map();
   }
 
   /// Set the deadline for the client call.
@@ -375,8 +377,8 @@ class ClientContext {
   mutable std::shared_ptr<const AuthContext> auth_context_;
   struct census_context* census_context_;
   std::multimap<grpc::string, grpc::string> send_initial_metadata_;
-  std::multimap<grpc::string_ref, grpc::string_ref> recv_initial_metadata_;
+  MetadataMap recv_initial_metadata_;
-  std::multimap<grpc::string_ref, grpc::string_ref> trailing_metadata_;
+  MetadataMap trailing_metadata_;
 
   grpc_call* propagate_from_call_;
   PropagationOptions propagation_options_;

include/grpc++/impl/codegen/metadata_map.h

@@ -0,0 +1,71 @@
+/*
+*
+* Copyright 2015, Google Inc.
+* All rights reserved.
+*
+* Redistribution and use in source and binary forms, with or without
+* modification, are permitted provided that the following conditions are
+* met:
+*
+*     * Redistributions of source code must retain the above copyright
+* notice, this list of conditions and the following disclaimer.
+*     * Redistributions in binary form must reproduce the above
+* copyright notice, this list of conditions and the following disclaimer
+* in the documentation and/or other materials provided with the
+* distribution.
+*     * Neither the name of Google Inc. nor the names of its
+* contributors may be used to endorse or promote products derived from
+* this software without specific prior written permission.
+*
+* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+*
+*/
+
+#ifndef META_H
+#define META_H
+
+#include <grpc++/impl/codegen/slice.h>
+
+namespace grpc {
+
+class MetadataMap {
+ public:
+  MetadataMap() { memset(&arr_, 0, sizeof(arr_)); }
+
+  ~MetadataMap() {
+    g_core_codegen_interface->grpc_metadata_array_destroy(&arr_);
+  }
+
+  void FillMap() {
+    for (size_t i = 0; i < arr_.count; i++) {
+      // TODO(yangg) handle duplicates?
+      map_.insert(std::pair<grpc::string_ref, grpc::string_ref>(
+          StringRefFromSlice(&arr_.metadata[i].key),
+          StringRefFromSlice(&arr_.metadata[i].value)));
+    }
+  }
+
+  std::multimap<grpc::string_ref, grpc::string_ref> *map() { return &map_; }
+  const std::multimap<grpc::string_ref, grpc::string_ref> *map() const {
+    return &map_;
+  }
+  grpc_metadata_array *arr() { return &arr_; }
+
+ private:
+  grpc_metadata_array arr_;
+  std::multimap<grpc::string_ref, grpc::string_ref> map_;
+};
+
+}  // namespace grpc
+
+#endif

include/grpc++/impl/codegen/server_context.h

@@ -39,6 +39,7 @@
 
 #include <grpc++/impl/codegen/config.h>
 #include <grpc++/impl/codegen/create_auth_context.h>
+#include <grpc++/impl/codegen/metadata_map.h>
 #include <grpc++/impl/codegen/security/auth_context.h>
 #include <grpc++/impl/codegen/string_ref.h>
 #include <grpc++/impl/codegen/time.h>
@@ -123,7 +124,7 @@ class ServerContext {
 
   const std::multimap<grpc::string_ref, grpc::string_ref>& client_metadata()
       const {
-    return client_metadata_;
+    return *client_metadata_.map();
   }
 
   grpc_compression_level compression_level() const {
@@ -223,7 +224,7 @@ class ServerContext {
   CompletionQueue* cq_;
   bool sent_initial_metadata_;
   mutable std::shared_ptr<const AuthContext> auth_context_;
-  std::multimap<grpc::string_ref, grpc::string_ref> client_metadata_;
+  MetadataMap client_metadata_;
   std::multimap<grpc::string, grpc::string> initial_metadata_;
   std::multimap<grpc::string, grpc::string> trailing_metadata_;
 

include/grpc++/impl/codegen/slice.h

@@ -39,9 +39,10 @@
 
 namespace grpc {
 
-inline grpc::string_ref StringRefFromSlice(grpc_slice slice) {
+inline grpc::string_ref StringRefFromSlice(const grpc_slice* slice) {
-  return grpc::string_ref(reinterpret_cast<char*>(GRPC_SLICE_START_PTR(slice)),
+  return grpc::string_ref(
-                          GRPC_SLICE_LENGTH(slice));
+      reinterpret_cast<const char*>(GRPC_SLICE_START_PTR(*slice)),
+      GRPC_SLICE_LENGTH(*slice));
 }
 
 inline grpc::string StringFromCopiedSlice(grpc_slice slice) {
@@ -61,4 +62,4 @@ inline grpc_slice SliceFromCopiedString(const grpc::string& str) {
 
 }  // namespace grpc
 
-#endif
+#endif  // GRPCXX_IMPL_CODEGEN_SLICE_H

src/core/ext/census/gen/census.pb.h

@@ -292,4 +292,4 @@ extern const pb_field_t google_census_Metric_fields[5];
 } /* extern "C" */
 #endif
 
-#endif
+#endif /* GRPC_CORE_EXT_CENSUS_GEN_CENSUS_PB_H */

src/core/ext/census/gen/trace_context.pb.h

@@ -96,4 +96,4 @@ extern const pb_field_t google_trace_TraceContext_fields[4];
 } /* extern "C" */
 #endif
 
-#endif
+#endif /* GRPC_CORE_EXT_CENSUS_GEN_TRACE_CONTEXT_PB_H */

src/core/lib/compression/compression.c

@@ -84,8 +84,7 @@ int grpc_compression_algorithm_name(grpc_compression_algorithm algorithm,
 grpc_compression_algorithm grpc_compression_algorithm_from_slice(
     grpc_slice str) {
   if (grpc_slice_eq(str, GRPC_MDSTR_IDENTITY)) return GRPC_COMPRESS_NONE;
-  if (grpc_slice_eq(str, GRPC_MDSTR_DEFLATE))
+  if (grpc_slice_eq(str, GRPC_MDSTR_DEFLATE)) return GRPC_COMPRESS_DEFLATE;
-    return GRPC_COMPRESS_DEFLATE;
   if (grpc_slice_eq(str, GRPC_MDSTR_GZIP)) return GRPC_COMPRESS_GZIP;
   return GRPC_COMPRESS_ALGORITHMS_COUNT;
 }

src/core/lib/slice/slice_hash_table.h

@@ -29,8 +29,8 @@
  * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef GRPC_CORE_LIB_TRANSPORT_MDSTR_HASH_TABLE_H
+#ifndef GRPC_CORE_LIB_SLICE_SLICE_HASH_TABLE_H
-#define GRPC_CORE_LIB_TRANSPORT_MDSTR_HASH_TABLE_H
+#define GRPC_CORE_LIB_SLICE_SLICE_HASH_TABLE_H
 
 #include "src/core/lib/transport/metadata.h"
 
@@ -75,4 +75,4 @@ void grpc_slice_hash_table_unref(grpc_exec_ctx *exec_ctx,
 void *grpc_slice_hash_table_get(const grpc_slice_hash_table *table,
                                 const grpc_slice key);
 
-#endif /* GRPC_CORE_LIB_TRANSPORT_MDSTR_HASH_TABLE_H */
+#endif /* GRPC_CORE_LIB_SLICE_SLICE_HASH_TABLE_H */

src/core/lib/slice/slice_intern.c

@@ -189,22 +189,24 @@ uint32_t grpc_slice_hash(grpc_slice s) {
                             : s.refcount->vtable->hash(s);
 }
 
-void grpc_slice_static_intern(grpc_slice *slice) {
+grpc_slice grpc_slice_maybe_static_intern(grpc_slice slice,
-  if (GRPC_IS_STATIC_METADATA_STRING(*slice)) {
+                                          bool *returned_slice_is_different) {
-    return;
+  if (GRPC_IS_STATIC_METADATA_STRING(slice)) {
+    return slice;
   }
 
-  uint32_t hash = grpc_slice_hash(*slice);
+  uint32_t hash = grpc_slice_hash(slice);
   for (uint32_t i = 0; i <= max_static_metadata_hash_probe; i++) {
     static_metadata_hash_ent ent =
         static_metadata_hash[(hash + i) % GPR_ARRAY_SIZE(static_metadata_hash)];
     if (ent.hash == hash && ent.idx < GRPC_STATIC_MDSTR_COUNT &&
-        grpc_slice_eq(grpc_static_slice_table[ent.idx], *slice)) {
+        grpc_slice_eq(grpc_static_slice_table[ent.idx], slice)) {
-      grpc_slice_unref(*slice);
+      *returned_slice_is_different = true;
-      *slice = grpc_static_slice_table[ent.idx];
+      return grpc_static_slice_table[ent.idx];
-      return;
     }
   }
+
+  return slice;
 }
 
 bool grpc_slice_is_interned(grpc_slice slice) {

src/core/lib/slice/slice_internal.h

@@ -52,9 +52,12 @@ bool grpc_slice_is_interned(grpc_slice slice);
 void grpc_slice_intern_init(void);
 void grpc_slice_intern_shutdown(void);
 void grpc_test_only_set_slice_hash_seed(uint32_t key);
-// if slice matches a static slice, consume it and replace it with the static
+// if slice matches a static slice, returns the static slice
-// slice, otherwise do nothing: this is a fast interning for well known strings
+// otherwise returns the passed in slice (without reffing it)
-void grpc_slice_static_intern(grpc_slice *slice);
+// used for surface boundaries where we might receive an un-interned static
+// string
+grpc_slice grpc_slice_maybe_static_intern(grpc_slice slice,
+                                          bool *returned_slice_is_different);
 uint32_t grpc_static_slice_hash(grpc_slice s);
 int grpc_static_slice_eq(grpc_slice a, grpc_slice b);
 

src/core/lib/slice/slice_traits.h

@@ -31,8 +31,8 @@
  *
  */
 
-#ifndef SLICE_TRAITS_H
+#ifndef GRPC_CORE_LIB_SLICE_SLICE_TRAITS_H
-#define SLICE_TRAITS_H
+#define GRPC_CORE_LIB_SLICE_SLICE_TRAITS_H
 
 #include <grpc/slice.h>
 #include <stdbool.h>
@@ -41,4 +41,4 @@ bool grpc_slice_is_legal_header(grpc_slice s);
 bool grpc_slice_is_legal_nonbin_header(grpc_slice s);
 bool grpc_slice_is_bin_suffixed(grpc_slice s);
 
-#endif
+#endif /* GRPC_CORE_LIB_SLICE_SLICE_TRAITS_H */

src/core/lib/transport/metadata.c

@@ -342,8 +342,13 @@ grpc_mdelem grpc_mdelem_from_slices(grpc_exec_ctx *exec_ctx, grpc_slice key,
 
 grpc_mdelem grpc_mdelem_from_grpc_metadata(grpc_exec_ctx *exec_ctx,
                                            grpc_metadata *metadata) {
-  return grpc_mdelem_create(exec_ctx, metadata->key, metadata->value,
+  bool changed = false;
-                            (grpc_mdelem_data *)metadata);
+  grpc_slice key_slice =
+      grpc_slice_maybe_static_intern(metadata->key, &changed);
+  grpc_slice value_slice =
+      grpc_slice_maybe_static_intern(metadata->value, &changed);
+  return grpc_mdelem_create(exec_ctx, key_slice, value_slice,
+                            changed ? NULL : (grpc_mdelem_data *)metadata);
 }
 
 static size_t get_base64_encoded_size(size_t raw_length) {

src/cpp/server/secure_server_credentials.cc

@@ -72,8 +72,8 @@ void AuthMetadataProcessorAyncWrapper::InvokeProcessor(
     grpc_process_auth_metadata_done_cb cb, void* user_data) {
   AuthMetadataProcessor::InputMetadata metadata;
   for (size_t i = 0; i < num_md; i++) {
-    metadata.insert(std::make_pair(StringRefFromSlice(md[i].key),
+    metadata.insert(std::make_pair(StringRefFromSlice(&md[i].key),
-                                   StringRefFromSlice(md[i].value)));
+                                   StringRefFromSlice(&md[i].value)));
   }
   SecureAuthContext context(ctx, false);
   AuthMetadataProcessor::OutputMetadata consumed_metadata;

src/cpp/server/server_cc.cc

@@ -586,14 +586,8 @@ ServerInterface::BaseAsyncRequest::~BaseAsyncRequest() {
 bool ServerInterface::BaseAsyncRequest::FinalizeResult(void** tag,
                                                        bool* status) {
   if (*status) {
-    for (size_t i = 0; i < initial_metadata_array_.count; i++) {
+    context_->client_metadata_.FillMap();
-      context_->client_metadata_.insert(
-          std::pair<grpc::string_ref, grpc::string_ref>(
-              StringRefFromSlice(initial_metadata_array_.metadata[i].key),
-              StringRefFromSlice(initial_metadata_array_.metadata[i].value)));
   }
-  }
-  grpc_metadata_array_destroy(&initial_metadata_array_);
   context_->set_call(call_);
   context_->cq_ = call_cq_;
   Call call(call_, server_, call_cq_, server_->max_receive_message_size());
@@ -619,8 +613,8 @@ void ServerInterface::RegisteredAsyncRequest::IssueRequest(
     ServerCompletionQueue* notification_cq) {
   grpc_server_request_registered_call(
       server_->server(), registered_method, &call_, &context_->deadline_,
-      &initial_metadata_array_, payload, call_cq_->cq(), notification_cq->cq(),
+      context_->client_metadata_.arr(), payload, call_cq_->cq(),
-      this);
+      notification_cq->cq(), this);
 }
 
 ServerInterface::GenericAsyncRequest::GenericAsyncRequest(

src/cpp/server/server_context.cc

@@ -144,9 +144,10 @@ ServerContext::ServerContext(gpr_timespec deadline, grpc_metadata* metadata,
       sent_initial_metadata_(false),
       compression_level_set_(false) {
   for (size_t i = 0; i < metadata_count; i++) {
-    client_metadata_.insert(std::pair<grpc::string_ref, grpc::string_ref>(
+    client_metadata_.map()->insert(
-        StringRefFromSlice(metadata[i].key),
+        std::pair<grpc::string_ref, grpc::string_ref>(
-        StringRefFromSlice(metadata[i].value)));
+            StringRefFromSlice(&metadata[i].key),
+            StringRefFromSlice(&metadata[i].value)));
   }
 }
 

src/cpp/test/server_context_test_spouse.cc

@@ -40,11 +40,12 @@ void ServerContextTestSpouse::AddClientMetadata(const grpc::string& key,
                                                 const grpc::string& value) {
   client_metadata_storage_.insert(
       std::pair<grpc::string, grpc::string>(key, value));
-  ctx_->client_metadata_.clear();
+  ctx_->client_metadata_.map()->clear();
   for (auto iter = client_metadata_storage_.begin();
        iter != client_metadata_storage_.end(); ++iter) {
-    ctx_->client_metadata_.insert(std::pair<grpc::string_ref, grpc::string_ref>(
+    ctx_->client_metadata_.map()->insert(
-        iter->first.c_str(), iter->second.c_str()));
+        std::pair<grpc::string_ref, grpc::string_ref>(iter->first.c_str(),
+                                                      iter->second.c_str()));
   }
 }
 

test/core/end2end/cq_verifier.c

@@ -109,8 +109,7 @@ static int has_metadata_slices(const grpc_metadata *md, size_t count,
                                grpc_slice key, grpc_slice value) {
   size_t i;
   for (i = 0; i < count; i++) {
-    if (grpc_slice_eq(md[i].key, key) &&
+    if (grpc_slice_eq(md[i].key, key) && grpc_slice_eq(md[i].value, value)) {
-        grpc_slice_eq(md[i].value, value)) {
       return 1;
     }
   }

test/core/end2end/fixtures/h2_ssl_cert.c

@@ -323,8 +323,8 @@ static void simple_request_body(grpc_end2end_test_fixture f,
 
   grpc_slice host = grpc_slice_from_static_string("foo.test.google.fr:1234");
   c = grpc_channel_create_call(f.client, NULL, GRPC_PROPAGATE_DEFAULTS, f.cq,
-                               grpc_slice_from_static_string("/foo"), &host, deadline,
+                               grpc_slice_from_static_string("/foo"), &host,
-                               NULL);
+                               deadline, NULL);
   GPR_ASSERT(c);
 
   memset(ops, 0, sizeof(ops));

test/core/slice/percent_encoding_test.c

@@ -123,8 +123,8 @@ static void test_nonconformant_vector(const char *encoded,
           encoded2raw_permissive_msg);
   gpr_free(encoded2raw_permissive_msg);
 
-  GPR_ASSERT(grpc_slice_eq(permissive_unencoded_slice,
+  GPR_ASSERT(
-                                 encoded2raw_permissive_slice));
+      grpc_slice_eq(permissive_unencoded_slice, encoded2raw_permissive_slice));
 
   grpc_slice_unref(permissive_unencoded_slice);
   grpc_slice_unref(encoded2raw_permissive_slice);

test/cpp/end2end/async_end2end_test.cc

@@ -228,12 +228,7 @@ class TestScenario {
       : disable_blocking(non_block),
         credentials_type(creds_type),
         message_content(content) {}
-  void Log() const {
+  void Log() const;
-    gpr_log(
-        GPR_INFO,
-        "Scenario: disable_blocking %d, credentials %s, message size %" PRIuPTR,
-        disable_blocking, credentials_type.c_str(), message_content.size());
-  }
   bool disable_blocking;
   // Although the below grpc::string's are logically const, we can't declare
   // them const because of a limitation in the way old compilers (e.g., gcc-4.4)
@@ -242,6 +237,20 @@ class TestScenario {
   grpc::string message_content;
 };
 
+static std::ostream& operator<<(std::ostream& out,
+                                const TestScenario& scenario) {
+  return out << "TestScenario{disable_blocking="
+             << (scenario.disable_blocking ? "true" : "false")
+             << ", credentials='" << scenario.credentials_type
+             << "', message_size=" << scenario.message_content.size() << "}";
+}
+
+void TestScenario::Log() const {
+  std::ostringstream out;
+  out << *this;
+  gpr_log(GPR_DEBUG, "%s", out.str().c_str());
+}
+
 class AsyncEnd2endTest : public ::testing::TestWithParam<TestScenario> {
  protected:
   AsyncEnd2endTest() { GetParam().Log(); }

test/cpp/end2end/end2end_test.cc

@@ -209,10 +209,7 @@ class TestScenario {
  public:
   TestScenario(bool proxy, const grpc::string& creds_type)
       : use_proxy(proxy), credentials_type(creds_type) {}
-  void Log() const {
+  void Log() const;
-    gpr_log(GPR_INFO, "Scenario: proxy %d, credentials %s", use_proxy,
-            credentials_type.c_str());
-  }
   bool use_proxy;
   // Although the below grpc::string is logically const, we can't declare
   // them const because of a limitation in the way old compilers (e.g., gcc-4.4)
@@ -220,6 +217,19 @@ class TestScenario {
   grpc::string credentials_type;
 };
 
+static std::ostream& operator<<(std::ostream& out,
+                                const TestScenario& scenario) {
+  return out << "TestScenario{use_proxy="
+             << (scenario.use_proxy ? "true" : "false") << ", credentials='"
+             << scenario.credentials_type << "'}";
+}
+
+void TestScenario::Log() const {
+  std::ostringstream out;
+  out << *this;
+  gpr_log(GPR_DEBUG, "%s", out.str().c_str());
+}
+
 class End2endTest : public ::testing::TestWithParam<TestScenario> {
  protected:
   End2endTest()
@@ -635,7 +645,7 @@ TEST_P(End2endServerTryCancelTest, BidiStreamServerCancelAfter) {
   TestBidiStreamServerCancel(CANCEL_AFTER_PROCESSING, 5);
 }
 
-TEST_P(End2endTest, SimpleRpcWithCustomeUserAgentPrefix) {
+TEST_P(End2endTest, SimpleRpcWithCustomUserAgentPrefix) {
   user_agent_prefix_ = "custom_prefix";
   ResetStub();
   EchoRequest request;

test/cpp/grpclb/grpclb_test.cc

@@ -346,9 +346,8 @@ static void start_backend_server(server_fixture *sf) {
     }
     GPR_ASSERT(ev.type == GRPC_OP_COMPLETE);
     const string expected_token =
-        strlen(sf->lb_token_prefix) == 0
+        strlen(sf->lb_token_prefix) == 0 ? "" : sf->lb_token_prefix +
-            ? ""
+                                                    std::to_string(sf->port);
-            : sf->lb_token_prefix + std::to_string(sf->port);
     GPR_ASSERT(contains_metadata(&request_metadata_recv, "lb-token",
                                  expected_token.c_str()));