Update by review

include/grpcpp/server_builder.h

@@ -173,7 +173,7 @@ class ServerBuilder {
   /// The service must exist for the lifetime of the \a Server instance
   /// returned by \a BuildAndStart(). Only matches requests with :authority \a
   /// host
-  ServerBuilder& RegisterService(const std::string& addr,
+  ServerBuilder& RegisterService(const std::string& host,
                                  grpc::Service* service);
 
   /// Register a generic service.

src/core/lib/channel/channel_args.h

@@ -33,7 +33,6 @@
 grpc_channel_args* grpc_channel_args_copy(const grpc_channel_args* src);
 
 /** Copy the arguments in \a src into a new instance, stably sorting keys */
-
 grpc_channel_args* grpc_channel_args_normalize(const grpc_channel_args* src);
 
 /** Copy the arguments in \a src and append \a to_add. If \a to_add is NULL, it

src/core/lib/iomgr/load_file.h

@@ -27,7 +27,7 @@
 
 #include "src/core/lib/iomgr/error.h"
 
-/* Loads the content of a file into an output. add_null_terminator will add
+/* Loads the content of a file into a slice. add_null_terminator will add
    a NULL terminator if non-zero. */
 grpc_error* grpc_load_file(const char* filename, int add_null_terminator,
                            grpc_slice* output);

src/core/lib/surface/call.cc

@@ -284,7 +284,8 @@ grpc_core::TraceFlag grpc_compression_trace(false, "compression");
 #define CALL_FROM_TOP_ELEM(top_elem) \
   CALL_FROM_CALL_STACK(grpc_call_stack_from_top_element(top_elem))
 
-static void execute_batch(grpc_call* call, grpc_transport_stream_op_batch* op,
+static void execute_batch(grpc_call* call,
+                          grpc_transport_stream_op_batch* batch,
                           grpc_closure* start_batch_closure);
 
 static void cancel_with_status(grpc_call* c, grpc_status_code status,
@@ -637,10 +638,11 @@ static void execute_batch_in_call_combiner(void* arg, grpc_error* /*ignored*/) {
 
 // start_batch_closure points to a caller-allocated closure to be used
 // for entering the call combiner.
-static void execute_batch(grpc_call* call, grpc_transport_stream_op_batch* op,
+static void execute_batch(grpc_call* call,
+                          grpc_transport_stream_op_batch* batch,
                           grpc_closure* start_batch_closure) {
-  op->handler_private.extra_arg = call;
-  GRPC_CLOSURE_INIT(start_batch_closure, execute_batch_in_call_combiner, op,
+  batch->handler_private.extra_arg = call;
+  GRPC_CLOSURE_INIT(start_batch_closure, execute_batch_in_call_combiner, batch,
                     grpc_schedule_on_exec_ctx);
   GRPC_CALL_COMBINER_START(&call->call_combiner, start_batch_closure,
                            GRPC_ERROR_NONE, "executing batch");

src/core/lib/transport/transport.cc

@@ -177,31 +177,33 @@ grpc_endpoint* grpc_transport_get_endpoint(grpc_transport* transport) {
 // though it lives in lib, it handles transport stream ops sure
 // it's grpc_transport_stream_op_batch_finish_with_failure
 void grpc_transport_stream_op_batch_finish_with_failure(
-    grpc_transport_stream_op_batch* op, grpc_error* error,
+    grpc_transport_stream_op_batch* batch, grpc_error* error,
     grpc_core::CallCombiner* call_combiner) {
-  if (op->send_message) {
-    op->payload->send_message.send_message.reset();
+  if (batch->send_message) {
+    batch->payload->send_message.send_message.reset();
   }
-  if (op->cancel_stream) {
-    GRPC_ERROR_UNREF(op->payload->cancel_stream.cancel_error);
+  if (batch->cancel_stream) {
+    GRPC_ERROR_UNREF(batch->payload->cancel_stream.cancel_error);
   }
   // Construct a list of closures to execute.
   grpc_core::CallCombinerClosureList closures;
-  if (op->recv_initial_metadata) {
-    closures.Add(op->payload->recv_initial_metadata.recv_initial_metadata_ready,
-                 GRPC_ERROR_REF(error), "failing recv_initial_metadata_ready");
+  if (batch->recv_initial_metadata) {
+    closures.Add(
+        batch->payload->recv_initial_metadata.recv_initial_metadata_ready,
+        GRPC_ERROR_REF(error), "failing recv_initial_metadata_ready");
   }
-  if (op->recv_message) {
-    closures.Add(op->payload->recv_message.recv_message_ready,
+  if (batch->recv_message) {
+    closures.Add(batch->payload->recv_message.recv_message_ready,
                  GRPC_ERROR_REF(error), "failing recv_message_ready");
   }
-  if (op->recv_trailing_metadata) {
+  if (batch->recv_trailing_metadata) {
     closures.Add(
-        op->payload->recv_trailing_metadata.recv_trailing_metadata_ready,
+        batch->payload->recv_trailing_metadata.recv_trailing_metadata_ready,
         GRPC_ERROR_REF(error), "failing recv_trailing_metadata_ready");
   }
-  if (op->on_complete != nullptr) {
-    closures.Add(op->on_complete, GRPC_ERROR_REF(error), "failing on_complete");
+  if (batch->on_complete != nullptr) {
+    closures.Add(batch->on_complete, GRPC_ERROR_REF(error),
+                 "failing on_complete");
   }
   // Execute closures.
   closures.RunClosures(call_combiner);
@@ -224,11 +226,11 @@ static void destroy_made_transport_op(void* arg, grpc_error* error) {
   delete op;
 }
 
-grpc_transport_op* grpc_make_transport_op(grpc_closure* on_consumed) {
+grpc_transport_op* grpc_make_transport_op(grpc_closure* on_complete) {
   made_transport_op* op = new made_transport_op();
   GRPC_CLOSURE_INIT(&op->outer_on_complete, destroy_made_transport_op, op,
                     grpc_schedule_on_exec_ctx);
-  op->inner_on_complete = on_consumed;
+  op->inner_on_complete = on_complete;
   op->op.on_consumed = &op->outer_on_complete;
   return &op->op;
 }
@@ -247,13 +249,13 @@ static void destroy_made_transport_stream_op(void* arg, grpc_error* error) {
 }
 
 grpc_transport_stream_op_batch* grpc_make_transport_stream_op(
-    grpc_closure* on_consumed) {
+    grpc_closure* on_complete) {
   made_transport_stream_op* op =
       static_cast<made_transport_stream_op*>(gpr_zalloc(sizeof(*op)));
   op->op.payload = &op->payload;
   GRPC_CLOSURE_INIT(&op->outer_on_complete, destroy_made_transport_stream_op,
                     op, grpc_schedule_on_exec_ctx);
-  op->inner_on_complete = on_consumed;
+  op->inner_on_complete = on_complete;
   op->op.on_complete = &op->outer_on_complete;
   return &op->op;
 }

src/core/lib/transport/transport.h

@@ -411,7 +411,7 @@ void grpc_transport_destroy_stream(grpc_transport* transport,
                                    grpc_closure* then_schedule_closure);
 
 void grpc_transport_stream_op_batch_finish_with_failure(
-    grpc_transport_stream_op_batch* op, grpc_error* error,
+    grpc_transport_stream_op_batch* batch, grpc_error* error,
     grpc_core::CallCombiner* call_combiner);
 
 std::string grpc_transport_stream_op_batch_string(
@@ -450,14 +450,14 @@ void grpc_transport_destroy(grpc_transport* transport);
 /* Get the endpoint used by \a transport */
 grpc_endpoint* grpc_transport_get_endpoint(grpc_transport* transport);
 
-/* Allocate a grpc_transport_op, and preconfigure the on_consumed closure to
-   \a on_consumed and then delete the returned transport op */
-grpc_transport_op* grpc_make_transport_op(grpc_closure* on_consumed);
-/* Allocate a grpc_transport_stream_op_batch, and preconfigure the on_consumed
+/* Allocate a grpc_transport_op, and preconfigure the on_complete closure to
+   \a on_complete and then delete the returned transport op */
+grpc_transport_op* grpc_make_transport_op(grpc_closure* on_complete);
+/* Allocate a grpc_transport_stream_op_batch, and preconfigure the on_complete
    closure
-   to \a on_consumed and then delete the returned transport op */
+   to \a on_complete and then delete the returned transport op */
 grpc_transport_stream_op_batch* grpc_make_transport_stream_op(
-    grpc_closure* on_consumed);
+    grpc_closure* on_complete);
 
 namespace grpc_core {
 // This is the key to be used for loading/storing keepalive_throttling in the

src/core/tsi/alts/handshaker/alts_tsi_handshaker.cc

@@ -253,8 +253,8 @@ static const tsi_handshaker_result_vtable result_vtable = {
 
 tsi_result alts_tsi_handshaker_result_create(grpc_gcp_HandshakerResp* resp,
                                              bool is_client,
-                                             tsi_handshaker_result** self) {
-  if (self == nullptr || resp == nullptr) {
+                                             tsi_handshaker_result** result) {
+  if (result == nullptr || resp == nullptr) {
     gpr_log(GPR_ERROR, "Invalid arguments to create_handshaker_result()");
     return TSI_INVALID_ARGUMENT;
   }
@@ -305,19 +305,19 @@ tsi_result alts_tsi_handshaker_result_create(grpc_gcp_HandshakerResp* resp,
       grpc_gcp_Identity_service_account(local_identity);
   // We don't check if local service account is empty here
   // because local identity could be empty in certain situations.
-  alts_tsi_handshaker_result* result =
-      static_cast<alts_tsi_handshaker_result*>(gpr_zalloc(sizeof(*result)));
-  result->key_data =
+  alts_tsi_handshaker_result* sresult =
+      static_cast<alts_tsi_handshaker_result*>(gpr_zalloc(sizeof(*sresult)));
+  sresult->key_data =
       static_cast<char*>(gpr_zalloc(kAltsAes128GcmRekeyKeyLength));
-  memcpy(result->key_data, key_data.data, kAltsAes128GcmRekeyKeyLength);
-  result->peer_identity =
+  memcpy(sresult->key_data, key_data.data, kAltsAes128GcmRekeyKeyLength);
+  sresult->peer_identity =
       static_cast<char*>(gpr_zalloc(peer_service_account.size + 1));
-  memcpy(result->peer_identity, peer_service_account.data,
+  memcpy(sresult->peer_identity, peer_service_account.data,
          peer_service_account.size);
-  result->max_frame_size = grpc_gcp_HandshakerResult_max_frame_size(hresult);
+  sresult->max_frame_size = grpc_gcp_HandshakerResult_max_frame_size(hresult);
   upb::Arena rpc_versions_arena;
   bool serialized = grpc_gcp_rpc_protocol_versions_encode(
-      peer_rpc_version, rpc_versions_arena.ptr(), &result->rpc_versions);
+      peer_rpc_version, rpc_versions_arena.ptr(), &sresult->rpc_versions);
   if (!serialized) {
     gpr_log(GPR_ERROR, "Failed to serialize peer's RPC protocol versions.");
     return TSI_FAILED_PRECONDITION;
@@ -363,11 +363,11 @@ tsi_result alts_tsi_handshaker_result_create(grpc_gcp_HandshakerResp* resp,
     gpr_log(GPR_ERROR, "Failed to serialize peer's ALTS context.");
     return TSI_FAILED_PRECONDITION;
   }
-  result->serialized_context =
+  sresult->serialized_context =
       grpc_slice_from_copied_buffer(serialized_ctx, serialized_ctx_length);
-  result->is_client = is_client;
-  result->base.vtable = &result_vtable;
-  *self = &result->base;
+  sresult->is_client = is_client;
+  sresult->base.vtable = &result_vtable;
+  *result = &sresult->base;
   return TSI_OK;
 }
 
@@ -652,21 +652,21 @@ tsi_result alts_tsi_handshaker_create(
   return TSI_OK;
 }
 
-void alts_tsi_handshaker_result_set_unused_bytes(tsi_handshaker_result* self,
+void alts_tsi_handshaker_result_set_unused_bytes(tsi_handshaker_result* result,
                                                  grpc_slice* recv_bytes,
                                                  size_t bytes_consumed) {
-  GPR_ASSERT(recv_bytes != nullptr && self != nullptr);
+  GPR_ASSERT(recv_bytes != nullptr && result != nullptr);
   if (GRPC_SLICE_LENGTH(*recv_bytes) == bytes_consumed) {
     return;
   }
-  alts_tsi_handshaker_result* result =
-      reinterpret_cast<alts_tsi_handshaker_result*>(self);
-  result->unused_bytes_size = GRPC_SLICE_LENGTH(*recv_bytes) - bytes_consumed;
-  result->unused_bytes =
-      static_cast<unsigned char*>(gpr_zalloc(result->unused_bytes_size));
-  memcpy(result->unused_bytes,
+  alts_tsi_handshaker_result* sresult =
+      reinterpret_cast<alts_tsi_handshaker_result*>(result);
+  sresult->unused_bytes_size = GRPC_SLICE_LENGTH(*recv_bytes) - bytes_consumed;
+  sresult->unused_bytes =
+      static_cast<unsigned char*>(gpr_zalloc(sresult->unused_bytes_size));
+  memcpy(sresult->unused_bytes,
          GRPC_SLICE_START_PTR(*recv_bytes) + bytes_consumed,
-         result->unused_bytes_size);
+         sresult->unused_bytes_size);
 }
 
 namespace grpc_core {

src/core/tsi/alts/handshaker/alts_tsi_handshaker.h

@@ -78,20 +78,20 @@ tsi_result alts_tsi_handshaker_create(
  * - resp: data received from the handshaker service.
  * - is_client: a boolean value indicating if the result belongs to a
  *   client or not.
- * - self: address of ALTS TSI handshaker result instance.
+ * - result: address of ALTS TSI handshaker result instance.
  */
 tsi_result alts_tsi_handshaker_result_create(grpc_gcp_HandshakerResp* resp,
                                              bool is_client,
-                                             tsi_handshaker_result** self);
+                                             tsi_handshaker_result** result);
 
 /**
  * This method sets unused bytes of ALTS TSI handshaker result instance.
  *
- * - self: an ALTS TSI handshaker result instance.
+ * - result: an ALTS TSI handshaker result instance.
  * - recv_bytes: data received from the handshaker service.
  * - bytes_consumed: size of data consumed by the handshaker service.
  */
-void alts_tsi_handshaker_result_set_unused_bytes(tsi_handshaker_result* self,
+void alts_tsi_handshaker_result_set_unused_bytes(tsi_handshaker_result* result,
                                                  grpc_slice* recv_bytes,
                                                  size_t bytes_consumed);
 

src/core/tsi/ssl_transport_security.cc

@@ -1150,11 +1150,11 @@ static const tsi_frame_protector_vtable frame_protector_vtable = {
 /* --- tsi_server_handshaker_factory methods implementation. --- */
 
 static void tsi_ssl_handshaker_factory_destroy(
-    tsi_ssl_handshaker_factory* self) {
-  if (self == nullptr) return;
+    tsi_ssl_handshaker_factory* factory) {
+  if (factory == nullptr) return;
 
-  if (self->vtable != nullptr && self->vtable->destroy != nullptr) {
-    self->vtable->destroy(self);
+  if (factory->vtable != nullptr && factory->vtable->destroy != nullptr) {
+    factory->vtable->destroy(factory);
   }
   /* Note, we don't free(self) here because this object is always directly
    * embedded in another object. If tsi_ssl_handshaker_factory_init allocates
@@ -1162,17 +1162,18 @@ static void tsi_ssl_handshaker_factory_destroy(
 }
 
 static tsi_ssl_handshaker_factory* tsi_ssl_handshaker_factory_ref(
-    tsi_ssl_handshaker_factory* self) {
-  if (self == nullptr) return nullptr;
-  gpr_refn(&self->refcount, 1);
-  return self;
+    tsi_ssl_handshaker_factory* factory) {
+  if (factory == nullptr) return nullptr;
+  gpr_refn(&factory->refcount, 1);
+  return factory;
 }
 
-static void tsi_ssl_handshaker_factory_unref(tsi_ssl_handshaker_factory* self) {
-  if (self == nullptr) return;
+static void tsi_ssl_handshaker_factory_unref(
+    tsi_ssl_handshaker_factory* factory) {
+  if (factory == nullptr) return;
 
-  if (gpr_unref(&self->refcount)) {
-    tsi_ssl_handshaker_factory_destroy(self);
+  if (gpr_unref(&factory->refcount)) {
+    tsi_ssl_handshaker_factory_destroy(factory);
   }
 }
 
@@ -1682,16 +1683,17 @@ static int select_protocol_list(const unsigned char** out,
 /* --- tsi_ssl_client_handshaker_factory methods implementation. --- */
 
 tsi_result tsi_ssl_client_handshaker_factory_create_handshaker(
-    tsi_ssl_client_handshaker_factory* self, const char* server_name_indication,
-    tsi_handshaker** handshaker) {
-  return create_tsi_ssl_handshaker(self->ssl_context, 1, server_name_indication,
-                                   &self->base, handshaker);
+    tsi_ssl_client_handshaker_factory* factory,
+    const char* server_name_indication, tsi_handshaker** handshaker) {
+  return create_tsi_ssl_handshaker(factory->ssl_context, 1,
+                                   server_name_indication, &factory->base,
+                                   handshaker);
 }
 
 void tsi_ssl_client_handshaker_factory_unref(
-    tsi_ssl_client_handshaker_factory* self) {
-  if (self == nullptr) return;
-  tsi_ssl_handshaker_factory_unref(&self->base);
+    tsi_ssl_client_handshaker_factory* factory) {
+  if (factory == nullptr) return;
+  tsi_ssl_handshaker_factory_unref(&factory->base);
 }
 
 static void tsi_ssl_client_handshaker_factory_destroy(
@@ -1718,18 +1720,18 @@ static int client_handshaker_factory_npn_callback(
 /* --- tsi_ssl_server_handshaker_factory methods implementation. --- */
 
 tsi_result tsi_ssl_server_handshaker_factory_create_handshaker(
-    tsi_ssl_server_handshaker_factory* self, tsi_handshaker** handshaker) {
-  if (self->ssl_context_count == 0) return TSI_INVALID_ARGUMENT;
+    tsi_ssl_server_handshaker_factory* factory, tsi_handshaker** handshaker) {
+  if (factory->ssl_context_count == 0) return TSI_INVALID_ARGUMENT;
   /* Create the handshaker with the first context. We will switch if needed
      because of SNI in ssl_server_handshaker_factory_servername_callback.  */
-  return create_tsi_ssl_handshaker(self->ssl_contexts[0], 0, nullptr,
-                                   &self->base, handshaker);
+  return create_tsi_ssl_handshaker(factory->ssl_contexts[0], 0, nullptr,
+                                   &factory->base, handshaker);
 }
 
 void tsi_ssl_server_handshaker_factory_unref(
-    tsi_ssl_server_handshaker_factory* self) {
-  if (self == nullptr) return;
-  tsi_ssl_handshaker_factory_unref(&self->base);
+    tsi_ssl_server_handshaker_factory* factory) {
+  if (factory == nullptr) return;
+  tsi_ssl_handshaker_factory_unref(&factory->base);
 }
 
 static void tsi_ssl_server_handshaker_factory_destroy(

src/core/tsi/ssl_transport_security.h

@@ -184,7 +184,7 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options(
     tsi_ssl_client_handshaker_factory** factory);
 
 /* Creates a client handshaker.
-  - self is the factory from which the handshaker will be created.
+  - factory is the factory from which the handshaker will be created.
   - server_name_indication indicates the name of the server the client is
     trying to connect to which will be relayed to the server using the SNI
     extension.
@@ -193,13 +193,13 @@ tsi_result tsi_create_ssl_client_handshaker_factory_with_options(
   - This method returns TSI_OK on success or TSI_INVALID_PARAMETER in the case
     where a parameter is invalid.  */
 tsi_result tsi_ssl_client_handshaker_factory_create_handshaker(
-    tsi_ssl_client_handshaker_factory* self, const char* server_name_indication,
-    tsi_handshaker** handshaker);
+    tsi_ssl_client_handshaker_factory* factory,
+    const char* server_name_indication, tsi_handshaker** handshaker);
 
 /* Decrements reference count of the handshaker factory. Handshaker factory will
  * be destroyed once no references exist. */
 void tsi_ssl_client_handshaker_factory_unref(
-    tsi_ssl_client_handshaker_factory* self);
+    tsi_ssl_client_handshaker_factory* factory);
 
 /* --- tsi_ssl_server_handshaker_factory object ---
 
@@ -315,18 +315,18 @@ tsi_result tsi_create_ssl_server_handshaker_factory_with_options(
     tsi_ssl_server_handshaker_factory** factory);
 
 /* Creates a server handshaker.
-  - self is the factory from which the handshaker will be created.
+  - factory is the factory from which the handshaker will be created.
   - handshaker is the address of the handshaker pointer to be created.
 
   - This method returns TSI_OK on success or TSI_INVALID_PARAMETER in the case
     where a parameter is invalid.  */
 tsi_result tsi_ssl_server_handshaker_factory_create_handshaker(
-    tsi_ssl_server_handshaker_factory* self, tsi_handshaker** handshaker);
+    tsi_ssl_server_handshaker_factory* factory, tsi_handshaker** handshaker);
 
 /* Decrements reference count of the handshaker factory. Handshaker factory will
  * be destroyed once no references exist. */
 void tsi_ssl_server_handshaker_factory_unref(
-    tsi_ssl_server_handshaker_factory* self);
+    tsi_ssl_server_handshaker_factory* factory);
 
 /* Util that checks that an ssl peer matches a specific name.
    Still TODO(jboeuf):

src/cpp/server/server_builder.cc

@@ -83,9 +83,9 @@ ServerBuilder& ServerBuilder::RegisterService(Service* service) {
   return *this;
 }
 
-ServerBuilder& ServerBuilder::RegisterService(const std::string& addr,
+ServerBuilder& ServerBuilder::RegisterService(const std::string& host,
                                               Service* service) {
-  services_.emplace_back(new NamedService(addr, service));
+  services_.emplace_back(new NamedService(host, service));
   return *this;
 }