From c2ef1103034bce7f5b6dcc18e5b13cd9047d1685 Mon Sep 17 00:00:00 2001 From: "Mark D. Roth" Date: Thu, 21 May 2020 08:47:41 -0700 Subject: [PATCH] Replace gpr_strvec with absl::StrJoin(). --- .../filters/client_channel/client_channel.cc | 9 +- .../client_channel/lb_policy/grpclb/grpclb.cc | 23 +-- .../client_channel/resolving_lb_policy.cc | 42 ++--- .../client_channel/resolving_lb_policy.h | 4 +- .../client_channel/xds/xds_bootstrap.cc | 69 ++++---- .../filters/http/client/http_client_filter.cc | 46 +++-- .../chttp2/transport/chttp2_transport.cc | 16 +- src/core/lib/channel/channel_args.cc | 27 +-- src/core/lib/channel/channel_args.h | 4 +- src/core/lib/channel/handshaker.cc | 24 ++- src/core/lib/debug/stats.cc | 48 +++--- src/core/lib/debug/stats.h | 2 +- src/core/lib/gpr/string.cc | 23 --- src/core/lib/gpr/string.h | 16 -- src/core/lib/http/format_request.cc | 111 +++++------- src/core/lib/iomgr/ev_epoll1_linux.cc | 38 ++-- .../credentials/oauth2/oauth2_credentials.cc | 45 +++-- src/core/lib/surface/call_log_batch.cc | 108 ++++++------ src/core/lib/surface/completion_queue.cc | 37 ++-- src/core/lib/surface/event_string.cc | 43 ++--- src/core/lib/surface/event_string.h | 4 +- src/core/lib/transport/transport.h | 5 +- src/core/lib/transport/transport_op_string.cc | 163 +++++++----------- .../grpcio/grpc/_cython/_cygrpc/iomgr.pxd.pxi | 2 +- test/core/bad_client/tests/large_metadata.cc | 21 +-- .../channel/minimal_stack_is_minimal_test.cc | 56 +++--- test/core/end2end/cq_verifier.cc | 78 ++++----- test/core/end2end/tests/simple_request.cc | 6 +- test/core/gpr/arena_test.cc | 22 ++- test/core/security/verify_jwt.cc | 5 +- test/core/util/cmdline.cc | 54 +++--- test/core/util/cmdline.h | 4 +- test/core/util/cmdline_test.cc | 17 +- 33 files changed, 482 insertions(+), 690 deletions(-) diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index 673546e84e2..11890a8894f 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -3164,10 +3164,9 @@ void CallData::OnComplete(void* arg, grpc_error* error) { ChannelData* chand = static_cast(elem->channel_data); CallData* calld = static_cast(elem->call_data); if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_call_trace)) { - char* batch_str = grpc_transport_stream_op_batch_string(&batch_data->batch); gpr_log(GPR_INFO, "chand=%p calld=%p: got on_complete, error=%s, batch=%s", - chand, calld, grpc_error_string(error), batch_str); - gpr_free(batch_str); + chand, calld, grpc_error_string(error), + grpc_transport_stream_op_batch_string(&batch_data->batch).c_str()); } SubchannelCallRetryState* retry_state = static_cast( @@ -3240,10 +3239,8 @@ void CallData::AddClosureForSubchannelBatch( GRPC_CLOSURE_INIT(&batch->handler_private.closure, StartBatchInCallCombiner, batch, grpc_schedule_on_exec_ctx); if (GRPC_TRACE_FLAG_ENABLED(grpc_client_channel_call_trace)) { - char* batch_str = grpc_transport_stream_op_batch_string(batch); gpr_log(GPR_INFO, "chand=%p calld=%p: starting subchannel batch: %s", chand, - this, batch_str); - gpr_free(batch_str); + this, grpc_transport_stream_op_batch_string(batch).c_str()); } closures->Add(&batch->handler_private.closure, GRPC_ERROR_NONE, "start_subchannel_batch"); diff --git a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc index 91f6e6ad946..d2423571295 100644 --- a/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc +++ b/src/core/ext/filters/client_channel/lb_policy/grpclb/grpclb.cc @@ -65,6 +65,8 @@ #include #include "absl/container/inlined_vector.h" +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" #include #include @@ -235,7 +237,7 @@ class GrpcLb : public LoadBalancingPolicy { const std::vector& serverlist() const { return serverlist_; } // Returns a text representation suitable for logging. - grpc_core::UniquePtr AsText() const; + std::string AsText() const; // Extracts all non-drop entries into a ServerAddressList. ServerAddressList GetServerAddressList( @@ -445,9 +447,8 @@ void ParseServer(const GrpcLbServer& server, grpc_resolved_address* addr) { } } -grpc_core::UniquePtr GrpcLb::Serverlist::AsText() const { - gpr_strvec entries; - gpr_strvec_init(&entries); +std::string GrpcLb::Serverlist::AsText() const { + std::vector entries; for (size_t i = 0; i < serverlist_.size(); ++i) { const GrpcLbServer& server = serverlist_[i]; std::string ipport; @@ -458,14 +459,10 @@ grpc_core::UniquePtr GrpcLb::Serverlist::AsText() const { ParseServer(server, &addr); ipport = grpc_sockaddr_to_string(&addr, false); } - char* entry; - gpr_asprintf(&entry, " %" PRIuPTR ": %s token=%s\n", i, ipport.c_str(), - server.load_balance_token); - gpr_strvec_add(&entries, entry); + entries.push_back(absl::StrFormat(" %" PRIuPTR ": %s token=%s\n", i, + ipport, server.load_balance_token)); } - grpc_core::UniquePtr result(gpr_strvec_flatten(&entries, nullptr)); - gpr_strvec_destroy(&entries); - return result; + return absl::StrJoin(entries, ""); } // vtables for channel args for LB token and client stats. @@ -1057,14 +1054,12 @@ void GrpcLb::BalancerCallState::OnBalancerMessageReceivedLocked() { auto serverlist_wrapper = MakeRefCounted(std::move(response.serverlist)); if (GRPC_TRACE_FLAG_ENABLED(grpc_lb_glb_trace)) { - grpc_core::UniquePtr serverlist_text = - serverlist_wrapper->AsText(); gpr_log(GPR_INFO, "[grpclb %p] lb_calld=%p: Serverlist with %" PRIuPTR " servers received:\n%s", grpclb_policy(), this, serverlist_wrapper->serverlist().size(), - serverlist_text.get()); + serverlist_wrapper->AsText().c_str()); } seen_serverlist_ = true; // Start sending client load report only after we start using the diff --git a/src/core/ext/filters/client_channel/resolving_lb_policy.cc b/src/core/ext/filters/client_channel/resolving_lb_policy.cc index bc69bae01cc..74a5c29968b 100644 --- a/src/core/ext/filters/client_channel/resolving_lb_policy.cc +++ b/src/core/ext/filters/client_channel/resolving_lb_policy.cc @@ -26,6 +26,9 @@ #include #include +#include "absl/strings/str_cat.h" +#include "absl/strings/str_join.h" + #include #include #include @@ -241,7 +244,6 @@ void ResolvingLoadBalancingPolicy::CreateOrUpdateLbPolicyLocked( } // Creates a new LB policy. -// Updates trace_strings to indicate what was done. OrphanablePtr ResolvingLoadBalancingPolicy::CreateLbPolicyLocked( const grpc_channel_args& args) { @@ -265,31 +267,21 @@ void ResolvingLoadBalancingPolicy::MaybeAddTraceMessagesForAddressChangesLocked( bool resolution_contains_addresses, TraceStringVector* trace_strings) { if (!resolution_contains_addresses && previous_resolution_contained_addresses_) { - trace_strings->push_back(gpr_strdup("Address list became empty")); + trace_strings->push_back("Address list became empty"); } else if (resolution_contains_addresses && !previous_resolution_contained_addresses_) { - trace_strings->push_back(gpr_strdup("Address list became non-empty")); + trace_strings->push_back("Address list became non-empty"); } previous_resolution_contained_addresses_ = resolution_contains_addresses; } void ResolvingLoadBalancingPolicy::ConcatenateAndAddChannelTraceLocked( - TraceStringVector* trace_strings) const { - if (!trace_strings->empty()) { - gpr_strvec v; - gpr_strvec_init(&v); - gpr_strvec_add(&v, gpr_strdup("Resolution event: ")); - bool is_first = 1; - for (size_t i = 0; i < trace_strings->size(); ++i) { - if (!is_first) gpr_strvec_add(&v, gpr_strdup(", ")); - is_first = false; - gpr_strvec_add(&v, (*trace_strings)[i]); - } - size_t len = 0; - grpc_core::UniquePtr message(gpr_strvec_flatten(&v, &len)); + const TraceStringVector& trace_strings) const { + if (!trace_strings.empty()) { + std::string message = + absl::StrCat("Resolution event: ", absl::StrJoin(trace_strings, ", ")); channel_control_helper()->AddTraceEvent(ChannelControlHelper::TRACE_INFO, - absl::string_view(message.get())); - gpr_strvec_destroy(&v); + message); } } @@ -314,7 +306,7 @@ void ResolvingLoadBalancingPolicy::OnResolverResultChangedLocked( // Process the resolver result. RefCountedPtr lb_policy_config; bool service_config_changed = false; - char* service_config_error_string = nullptr; + std::string service_config_error_string; if (process_resolver_result_ != nullptr) { grpc_error* service_config_error = GRPC_ERROR_NONE; bool no_valid_service_config = false; @@ -322,8 +314,7 @@ void ResolvingLoadBalancingPolicy::OnResolverResultChangedLocked( process_resolver_result_user_data_, result, &lb_policy_config, &service_config_error, &no_valid_service_config); if (service_config_error != GRPC_ERROR_NONE) { - service_config_error_string = - gpr_strdup(grpc_error_string(service_config_error)); + service_config_error_string = grpc_error_string(service_config_error); if (no_valid_service_config) { // We received an invalid service config and we don't have a // fallback service config. @@ -344,15 +335,14 @@ void ResolvingLoadBalancingPolicy::OnResolverResultChangedLocked( if (service_config_changed) { // TODO(ncteisen): might be worth somehow including a snippet of the // config in the trace, at the risk of bloating the trace logs. - trace_strings.push_back(gpr_strdup("Service config changed")); + trace_strings.push_back("Service config changed"); } - if (service_config_error_string != nullptr) { - trace_strings.push_back(service_config_error_string); - service_config_error_string = nullptr; + if (!service_config_error_string.empty()) { + trace_strings.push_back(service_config_error_string.c_str()); } MaybeAddTraceMessagesForAddressChangesLocked(resolution_contains_addresses, &trace_strings); - ConcatenateAndAddChannelTraceLocked(&trace_strings); + ConcatenateAndAddChannelTraceLocked(trace_strings); } } // namespace grpc_core diff --git a/src/core/ext/filters/client_channel/resolving_lb_policy.h b/src/core/ext/filters/client_channel/resolving_lb_policy.h index 8a04491424f..39815e28039 100644 --- a/src/core/ext/filters/client_channel/resolving_lb_policy.h +++ b/src/core/ext/filters/client_channel/resolving_lb_policy.h @@ -81,7 +81,7 @@ class ResolvingLoadBalancingPolicy : public LoadBalancingPolicy { void ResetBackoffLocked() override; private: - using TraceStringVector = absl::InlinedVector; + using TraceStringVector = absl::InlinedVector; class ResolverResultHandler; class ResolvingControlHelper; @@ -99,7 +99,7 @@ class ResolvingLoadBalancingPolicy : public LoadBalancingPolicy { void MaybeAddTraceMessagesForAddressChangesLocked( bool resolution_contains_addresses, TraceStringVector* trace_strings); void ConcatenateAndAddChannelTraceLocked( - TraceStringVector* trace_strings) const; + const TraceStringVector& trace_strings) const; void OnResolverResultChangedLocked(Resolver::Result result); // Passed in from caller at construction time. diff --git a/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc b/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc index d7d39c5b6f6..25ce4a7f5d1 100644 --- a/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc +++ b/src/core/ext/filters/client_channel/xds/xds_bootstrap.cc @@ -23,6 +23,8 @@ #include #include +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" #include "absl/strings/string_view.h" #include @@ -36,47 +38,36 @@ namespace grpc_core { namespace { -UniquePtr BootstrapString(const XdsBootstrap& bootstrap) { - gpr_strvec v; - gpr_strvec_init(&v); - char* tmp; +std::string BootstrapString(const XdsBootstrap& bootstrap) { + std::vector parts; if (bootstrap.node() != nullptr) { - gpr_asprintf(&tmp, - "node={\n" - " id=\"%s\",\n" - " cluster=\"%s\",\n" - " locality={\n" - " region=\"%s\",\n" - " zone=\"%s\",\n" - " subzone=\"%s\"\n" - " },\n" - " metadata=%s,\n" - "},\n", - bootstrap.node()->id.c_str(), - bootstrap.node()->cluster.c_str(), - bootstrap.node()->locality_region.c_str(), - bootstrap.node()->locality_zone.c_str(), - bootstrap.node()->locality_subzone.c_str(), - bootstrap.node()->metadata.Dump().c_str()); - gpr_strvec_add(&v, tmp); + parts.push_back(absl::StrFormat( + "node={\n" + " id=\"%s\",\n" + " cluster=\"%s\",\n" + " locality={\n" + " region=\"%s\",\n" + " zone=\"%s\",\n" + " subzone=\"%s\"\n" + " },\n" + " metadata=%s,\n" + "},\n", + bootstrap.node()->id, bootstrap.node()->cluster, + bootstrap.node()->locality_region, bootstrap.node()->locality_zone, + bootstrap.node()->locality_subzone, bootstrap.node()->metadata.Dump())); } - gpr_asprintf(&tmp, - "servers=[\n" - " {\n" - " uri=\"%s\",\n" - " creds=[\n", - bootstrap.server().server_uri.c_str()); - gpr_strvec_add(&v, tmp); - for (size_t i = 0; i < bootstrap.server().channel_creds.size(); ++i) { - const auto& creds = bootstrap.server().channel_creds[i]; - gpr_asprintf(&tmp, " {type=\"%s\", config=%s},\n", creds.type.c_str(), - creds.config.Dump().c_str()); - gpr_strvec_add(&v, tmp); + parts.push_back( + absl::StrFormat("servers=[\n" + " {\n" + " uri=\"%s\",\n" + " creds=[\n", + bootstrap.server().server_uri)); + for (const auto& creds : bootstrap.server().channel_creds) { + parts.push_back(absl::StrFormat(" {type=\"%s\", config=%s},\n", + creds.type, creds.config.Dump())); } - gpr_strvec_add(&v, gpr_strdup(" ]\n }\n]")); - UniquePtr result(gpr_strvec_flatten(&v, nullptr)); - gpr_strvec_destroy(&v); - return result; + parts.push_back(" ]\n }\n]"); + return absl::StrJoin(parts, ""); } } // namespace @@ -121,7 +112,7 @@ std::unique_ptr XdsBootstrap::ReadFromFile(XdsClient* client, if (*error == GRPC_ERROR_NONE && GRPC_TRACE_FLAG_ENABLED(*tracer)) { gpr_log(GPR_INFO, "[xds_client %p] Bootstrap config for creating xds client:\n%s", - client, BootstrapString(*result).get()); + client, BootstrapString(*result).c_str()); } return result; } diff --git a/src/core/ext/filters/http/client/http_client_filter.cc b/src/core/ext/filters/http/client/http_client_filter.cc index a0bea07c2fc..ef773b3a3ef 100644 --- a/src/core/ext/filters/http/client/http_client_filter.cc +++ b/src/core/ext/filters/http/client/http_client_filter.cc @@ -17,11 +17,19 @@ #include +#include +#include + +#include +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include -#include -#include + #include "src/core/ext/filters/http/client/http_client_filter.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/gprpp/manual_constructor.h" @@ -520,50 +528,36 @@ static size_t max_payload_size_from_args(const grpc_channel_args* args) { static grpc_core::ManagedMemorySlice user_agent_from_args( const grpc_channel_args* args, const char* transport_name) { - gpr_strvec v; - size_t i; - int is_first = 1; - char* tmp; + std::vector user_agent_fields; - gpr_strvec_init(&v); - - for (i = 0; args && i < args->num_args; i++) { + for (size_t i = 0; args && i < args->num_args; i++) { if (0 == strcmp(args->args[i].key, GRPC_ARG_PRIMARY_USER_AGENT_STRING)) { if (args->args[i].type != GRPC_ARG_STRING) { gpr_log(GPR_ERROR, "Channel argument '%s' should be a string", GRPC_ARG_PRIMARY_USER_AGENT_STRING); } else { - if (!is_first) gpr_strvec_add(&v, gpr_strdup(" ")); - is_first = 0; - gpr_strvec_add(&v, gpr_strdup(args->args[i].value.string)); + user_agent_fields.push_back(args->args[i].value.string); } } } - gpr_asprintf(&tmp, "%sgrpc-c/%s (%s; %s)", is_first ? "" : " ", - grpc_version_string(), GPR_PLATFORM_STRING, transport_name); - is_first = 0; - gpr_strvec_add(&v, tmp); + user_agent_fields.push_back( + absl::StrFormat("grpc-c/%s (%s; %s)", grpc_version_string(), + GPR_PLATFORM_STRING, transport_name)); - for (i = 0; args && i < args->num_args; i++) { + for (size_t i = 0; args && i < args->num_args; i++) { if (0 == strcmp(args->args[i].key, GRPC_ARG_SECONDARY_USER_AGENT_STRING)) { if (args->args[i].type != GRPC_ARG_STRING) { gpr_log(GPR_ERROR, "Channel argument '%s' should be a string", GRPC_ARG_SECONDARY_USER_AGENT_STRING); } else { - if (!is_first) gpr_strvec_add(&v, gpr_strdup(" ")); - is_first = 0; - gpr_strvec_add(&v, gpr_strdup(args->args[i].value.string)); + user_agent_fields.push_back(args->args[i].value.string); } } } - tmp = gpr_strvec_flatten(&v, nullptr); - gpr_strvec_destroy(&v); - grpc_core::ManagedMemorySlice result(tmp); - gpr_free(tmp); - - return result; + std::string user_agent_string = absl::StrJoin(user_agent_fields, " "); + return grpc_core::ManagedMemorySlice(user_agent_string.c_str()); } /* Constructor for channel_data */ diff --git a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc index 23985897b08..66c36e10274 100644 --- a/src/core/ext/transport/chttp2/transport/chttp2_transport.cc +++ b/src/core/ext/transport/chttp2/transport/chttp2_transport.cc @@ -1356,10 +1356,8 @@ static void perform_stream_op_locked(void* stream_op, s->context = op->payload->context; s->traced = op->is_traced; if (GRPC_TRACE_FLAG_ENABLED(grpc_http_trace)) { - char* str = grpc_transport_stream_op_batch_string(op); - gpr_log(GPR_INFO, "perform_stream_op_locked: %s; on_complete = %p", str, - op->on_complete); - gpr_free(str); + gpr_log(GPR_INFO, "perform_stream_op_locked: %s; on_complete = %p", + grpc_transport_stream_op_batch_string(op).c_str(), op->on_complete); if (op->send_initial_metadata) { log_metadata(op_payload->send_initial_metadata.send_initial_metadata, s->id, t->is_client, true); @@ -1654,9 +1652,8 @@ static void perform_stream_op(grpc_transport* gt, grpc_stream* gs, } if (GRPC_TRACE_FLAG_ENABLED(grpc_http_trace)) { - char* str = grpc_transport_stream_op_batch_string(op); - gpr_log(GPR_INFO, "perform_stream_op[s=%p]: %s", s, str); - gpr_free(str); + gpr_log(GPR_INFO, "perform_stream_op[s=%p]: %s", s, + grpc_transport_stream_op_batch_string(op).c_str()); } GRPC_CHTTP2_STREAM_REF(s, "perform_stream_op"); @@ -1845,9 +1842,8 @@ static void perform_transport_op_locked(void* stream_op, static void perform_transport_op(grpc_transport* gt, grpc_transport_op* op) { grpc_chttp2_transport* t = reinterpret_cast(gt); if (GRPC_TRACE_FLAG_ENABLED(grpc_http_trace)) { - char* msg = grpc_transport_op_string(op); - gpr_log(GPR_INFO, "perform_transport_op[t=%p]: %s", t, msg); - gpr_free(msg); + gpr_log(GPR_INFO, "perform_transport_op[t=%p]: %s", t, + grpc_transport_op_string(op).c_str()); } op->handler_private.extra_arg = gt; GRPC_CHTTP2_REF_TRANSPORT(t, "transport_op"); diff --git a/src/core/lib/channel/channel_args.cc b/src/core/lib/channel/channel_args.cc index 10b185f5962..4a3b053f2cd 100644 --- a/src/core/lib/channel/channel_args.cc +++ b/src/core/lib/channel/channel_args.cc @@ -21,6 +21,11 @@ #include #include +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include @@ -336,32 +341,28 @@ grpc_arg grpc_channel_arg_pointer_create( return arg; } -char* grpc_channel_args_string(const grpc_channel_args* args) { +std::string grpc_channel_args_string(const grpc_channel_args* args) { if (args == nullptr) return nullptr; - gpr_strvec v; - gpr_strvec_init(&v); + std::vector arg_strings; for (size_t i = 0; i < args->num_args; ++i) { const grpc_arg& arg = args->args[i]; - char* s; + std::string arg_string; switch (arg.type) { case GRPC_ARG_INTEGER: - gpr_asprintf(&s, "%s=%d", arg.key, arg.value.integer); + arg_string = absl::StrFormat("%s=%d", arg.key, arg.value.integer); break; case GRPC_ARG_STRING: - gpr_asprintf(&s, "%s=%s", arg.key, arg.value.string); + arg_string = absl::StrFormat("%s=%s", arg.key, arg.value.string); break; case GRPC_ARG_POINTER: - gpr_asprintf(&s, "%s=%p", arg.key, arg.value.pointer.p); + arg_string = absl::StrFormat("%s=%p", arg.key, arg.value.pointer.p); break; default: - gpr_asprintf(&s, "arg with unknown type"); + arg_string = "arg with unknown type"; } - gpr_strvec_add(&v, s); + arg_strings.push_back(arg_string); } - char* result = - gpr_strjoin_sep(const_cast(v.strs), v.count, ", ", nullptr); - gpr_strvec_destroy(&v); - return result; + return absl::StrJoin(arg_strings, ", "); } namespace { diff --git a/src/core/lib/channel/channel_args.h b/src/core/lib/channel/channel_args.h index df105fd944f..aed00891ae2 100644 --- a/src/core/lib/channel/channel_args.h +++ b/src/core/lib/channel/channel_args.h @@ -21,6 +21,8 @@ #include +#include + #include #include "src/core/lib/surface/channel_stack_type.h" @@ -116,7 +118,7 @@ grpc_arg grpc_channel_arg_pointer_create(char* name, void* value, // Returns a string representing channel args in human-readable form. // Callers takes ownership of result. -char* grpc_channel_args_string(const grpc_channel_args* args); +std::string grpc_channel_args_string(const grpc_channel_args* args); // Takes ownership of the old_args typedef grpc_channel_args* (*grpc_channel_args_client_channel_creation_mutator)( diff --git a/src/core/lib/channel/handshaker.cc b/src/core/lib/channel/handshaker.cc index 2bcc60abfa9..461a21db32f 100644 --- a/src/core/lib/channel/handshaker.cc +++ b/src/core/lib/channel/handshaker.cc @@ -20,6 +20,8 @@ #include +#include "absl/strings/str_format.h" + #include #include #include @@ -37,19 +39,16 @@ TraceFlag grpc_handshaker_trace(false, "handshaker"); namespace { -char* HandshakerArgsString(HandshakerArgs* args) { - char* args_str = grpc_channel_args_string(args->args); +std::string HandshakerArgsString(HandshakerArgs* args) { size_t num_args = args->args != nullptr ? args->args->num_args : 0; size_t read_buffer_length = args->read_buffer != nullptr ? args->read_buffer->length : 0; - char* str; - gpr_asprintf(&str, - "{endpoint=%p, args=%p {size=%" PRIuPTR - ": %s}, read_buffer=%p (length=%" PRIuPTR "), exit_early=%d}", - args->endpoint, args->args, num_args, args_str, - args->read_buffer, read_buffer_length, args->exit_early); - gpr_free(args_str); - return str; + return absl::StrFormat( + "{endpoint=%p, args=%p {size=%" PRIuPTR + ": %s}, read_buffer=%p (length=%" PRIuPTR "), exit_early=%d}", + args->endpoint, args->args, num_args, + grpc_channel_args_string(args->args), args->read_buffer, + read_buffer_length, args->exit_early); } } // namespace @@ -127,12 +126,11 @@ void HandshakeManager::Shutdown(grpc_error* why) { // Returns true if we've scheduled the on_handshake_done callback. bool HandshakeManager::CallNextHandshakerLocked(grpc_error* error) { if (GRPC_TRACE_FLAG_ENABLED(grpc_handshaker_trace)) { - char* args_str = HandshakerArgsString(&args_); gpr_log(GPR_INFO, "handshake_manager %p: error=%s shutdown=%d index=%" PRIuPTR ", args=%s", - this, grpc_error_string(error), is_shutdown_, index_, args_str); - gpr_free(args_str); + this, grpc_error_string(error), is_shutdown_, index_, + HandshakerArgsString(&args_).c_str()); } GPR_ASSERT(index_ <= handshakers_.size()); // If we got an error or we've been shut down or we're exiting early or diff --git a/src/core/lib/debug/stats.cc b/src/core/lib/debug/stats.cc index d8ddf03ac1c..7f602b6b9a1 100644 --- a/src/core/lib/debug/stats.cc +++ b/src/core/lib/debug/stats.cc @@ -23,6 +23,11 @@ #include #include +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include @@ -140,39 +145,28 @@ double grpc_stats_histo_percentile(const grpc_stats_data* stats, static_cast(count) * percentile / 100.0); } -char* grpc_stats_data_as_json(const grpc_stats_data* data) { - gpr_strvec v; - char* tmp; - bool is_first = true; - gpr_strvec_init(&v); - gpr_strvec_add(&v, gpr_strdup("{")); +std::string grpc_stats_data_as_json(const grpc_stats_data* data) { + std::vector parts; + parts.push_back("{"); for (size_t i = 0; i < GRPC_STATS_COUNTER_COUNT; i++) { - gpr_asprintf(&tmp, "%s\"%s\": %" PRIdPTR, is_first ? "" : ", ", - grpc_stats_counter_name[i], data->counters[i]); - gpr_strvec_add(&v, tmp); - is_first = false; + parts.push_back(absl::StrFormat( + "\"%s\": %" PRIdPTR, grpc_stats_counter_name[i], data->counters[i])); } for (size_t i = 0; i < GRPC_STATS_HISTOGRAM_COUNT; i++) { - gpr_asprintf(&tmp, "%s\"%s\": [", is_first ? "" : ", ", - grpc_stats_histogram_name[i]); - gpr_strvec_add(&v, tmp); + parts.push_back(absl::StrFormat("\"%s\": [", grpc_stats_histogram_name[i])); for (int j = 0; j < grpc_stats_histo_buckets[i]; j++) { - gpr_asprintf(&tmp, "%s%" PRIdPTR, j == 0 ? "" : ",", - data->histograms[grpc_stats_histo_start[i] + j]); - gpr_strvec_add(&v, tmp); + parts.push_back( + absl::StrFormat("%s%" PRIdPTR, j == 0 ? "" : ",", + data->histograms[grpc_stats_histo_start[i] + j])); } - gpr_asprintf(&tmp, "], \"%s_bkt\": [", grpc_stats_histogram_name[i]); - gpr_strvec_add(&v, tmp); + parts.push_back( + absl::StrFormat("], \"%s_bkt\": [", grpc_stats_histogram_name[i])); for (int j = 0; j < grpc_stats_histo_buckets[i]; j++) { - gpr_asprintf(&tmp, "%s%d", j == 0 ? "" : ",", - grpc_stats_histo_bucket_boundaries[i][j]); - gpr_strvec_add(&v, tmp); + parts.push_back(absl::StrFormat( + "%s%d", j == 0 ? "" : ",", grpc_stats_histo_bucket_boundaries[i][j])); } - gpr_strvec_add(&v, gpr_strdup("]")); - is_first = false; + parts.push_back("]"); } - gpr_strvec_add(&v, gpr_strdup("}")); - tmp = gpr_strvec_flatten(&v, nullptr); - gpr_strvec_destroy(&v); - return tmp; + parts.push_back("}"); + return absl::StrJoin(parts, ""); } diff --git a/src/core/lib/debug/stats.h b/src/core/lib/debug/stats.h index 9e88ad7000f..6117e8e4ff8 100644 --- a/src/core/lib/debug/stats.h +++ b/src/core/lib/debug/stats.h @@ -56,7 +56,7 @@ void grpc_stats_collect(grpc_stats_data* output); // c = b-a void grpc_stats_diff(const grpc_stats_data* b, const grpc_stats_data* a, grpc_stats_data* c); -char* grpc_stats_data_as_json(const grpc_stats_data* data); +std::string grpc_stats_data_as_json(const grpc_stats_data* data); int grpc_stats_histo_find_bucket_slow(int value, const int* table, int table_size); double grpc_stats_histo_percentile(const grpc_stats_data* data, diff --git a/src/core/lib/gpr/string.cc b/src/core/lib/gpr/string.cc index 14436ec1bf9..51f7168612e 100644 --- a/src/core/lib/gpr/string.cc +++ b/src/core/lib/gpr/string.cc @@ -266,29 +266,6 @@ char* gpr_strjoin_sep(const char** strs, size_t nstrs, const char* sep, return out; } -void gpr_strvec_init(gpr_strvec* sv) { memset(sv, 0, sizeof(*sv)); } - -void gpr_strvec_destroy(gpr_strvec* sv) { - size_t i; - for (i = 0; i < sv->count; i++) { - gpr_free(sv->strs[i]); - } - gpr_free(sv->strs); -} - -void gpr_strvec_add(gpr_strvec* sv, char* str) { - if (sv->count == sv->capacity) { - sv->capacity = GPR_MAX(sv->capacity + 8, sv->capacity * 2); - sv->strs = static_cast( - gpr_realloc(sv->strs, sizeof(char*) * sv->capacity)); - } - sv->strs[sv->count++] = str; -} - -char* gpr_strvec_flatten(gpr_strvec* sv, size_t* final_length) { - return gpr_strjoin((const char**)sv->strs, sv->count, final_length); -} - int gpr_strincmp(const char* a, const char* b, size_t n) { int ca, cb; do { diff --git a/src/core/lib/gpr/string.h b/src/core/lib/gpr/string.h index fcccf5e6764..b7d76986e5c 100644 --- a/src/core/lib/gpr/string.h +++ b/src/core/lib/gpr/string.h @@ -96,22 +96,6 @@ void gpr_string_split(const char* input, const char* sep, char*** strs, 0, 3, 6 or 9 fractional digits. */ char* gpr_format_timespec(gpr_timespec); -/* A vector of strings... for building up a final string one piece at a time */ -typedef struct { - char** strs; - size_t count; - size_t capacity; -} gpr_strvec; - -/* Initialize/destroy */ -void gpr_strvec_init(gpr_strvec* strs); -void gpr_strvec_destroy(gpr_strvec* strs); -/* Add a string to a strvec, takes ownership of the string */ -void gpr_strvec_add(gpr_strvec* strs, char* add); -/* Return a joined string with all added substrings, optionally setting - total_length as per gpr_strjoin */ -char* gpr_strvec_flatten(gpr_strvec* strs, size_t* total_length); - /** Case insensitive string comparison... return <0 if lower(a)0 if lower(a)>lower(b) */ int gpr_stricmp(const char* a, const char* b); diff --git a/src/core/lib/http/format_request.cc b/src/core/lib/http/format_request.cc index 17123446483..947b19cbfac 100644 --- a/src/core/lib/http/format_request.cc +++ b/src/core/lib/http/format_request.cc @@ -24,99 +24,80 @@ #include #include +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include #include "src/core/lib/gpr/string.h" static void fill_common_header(const grpc_httpcli_request* request, - gpr_strvec* buf, bool connection_close) { - size_t i; - gpr_strvec_add(buf, gpr_strdup(request->http.path)); - gpr_strvec_add(buf, gpr_strdup(" HTTP/1.0\r\n")); + bool connection_close, + std::vector* buf) { + buf->push_back(request->http.path); + buf->push_back(" HTTP/1.0\r\n"); /* just in case some crazy server really expects HTTP/1.1 */ - gpr_strvec_add(buf, gpr_strdup("Host: ")); - gpr_strvec_add(buf, gpr_strdup(request->host)); - gpr_strvec_add(buf, gpr_strdup("\r\n")); - if (connection_close) - gpr_strvec_add(buf, gpr_strdup("Connection: close\r\n")); - gpr_strvec_add(buf, - gpr_strdup("User-Agent: " GRPC_HTTPCLI_USER_AGENT "\r\n")); + buf->push_back("Host: "); + buf->push_back(request->host); + buf->push_back("\r\n"); + if (connection_close) buf->push_back("Connection: close\r\n"); + buf->push_back("User-Agent: " GRPC_HTTPCLI_USER_AGENT "\r\n"); /* user supplied headers */ - for (i = 0; i < request->http.hdr_count; i++) { - gpr_strvec_add(buf, gpr_strdup(request->http.hdrs[i].key)); - gpr_strvec_add(buf, gpr_strdup(": ")); - gpr_strvec_add(buf, gpr_strdup(request->http.hdrs[i].value)); - gpr_strvec_add(buf, gpr_strdup("\r\n")); + for (size_t i = 0; i < request->http.hdr_count; i++) { + buf->push_back(request->http.hdrs[i].key); + buf->push_back(": "); + buf->push_back(request->http.hdrs[i].value); + buf->push_back("\r\n"); } } grpc_slice grpc_httpcli_format_get_request( const grpc_httpcli_request* request) { - gpr_strvec out; - char* flat; - size_t flat_len; - - gpr_strvec_init(&out); - gpr_strvec_add(&out, gpr_strdup("GET ")); - fill_common_header(request, &out, true); - gpr_strvec_add(&out, gpr_strdup("\r\n")); - - flat = gpr_strvec_flatten(&out, &flat_len); - gpr_strvec_destroy(&out); - - return grpc_slice_new(flat, flat_len, gpr_free); + std::vector out; + out.push_back("GET "); + fill_common_header(request, true, &out); + out.push_back("\r\n"); + std::string req = absl::StrJoin(out, ""); + return grpc_slice_from_copied_buffer(req.data(), req.size()); } grpc_slice grpc_httpcli_format_post_request(const grpc_httpcli_request* request, const char* body_bytes, size_t body_size) { - gpr_strvec out; - char* tmp; - size_t out_len; - size_t i; - - gpr_strvec_init(&out); - - gpr_strvec_add(&out, gpr_strdup("POST ")); - fill_common_header(request, &out, true); - if (body_bytes) { - uint8_t has_content_type = 0; - for (i = 0; i < request->http.hdr_count; i++) { + std::vector out; + out.push_back("POST "); + fill_common_header(request, true, &out); + if (body_bytes != nullptr) { + bool has_content_type = false; + for (size_t i = 0; i < request->http.hdr_count; i++) { if (strcmp(request->http.hdrs[i].key, "Content-Type") == 0) { - has_content_type = 1; + has_content_type = true; break; } } if (!has_content_type) { - gpr_strvec_add(&out, gpr_strdup("Content-Type: text/plain\r\n")); + out.push_back("Content-Type: text/plain\r\n"); } - gpr_asprintf(&tmp, "Content-Length: %lu\r\n", - static_cast(body_size)); - gpr_strvec_add(&out, tmp); + out.push_back(absl::StrFormat("Content-Length: %lu\r\n", + static_cast(body_size))); } - gpr_strvec_add(&out, gpr_strdup("\r\n")); - tmp = gpr_strvec_flatten(&out, &out_len); - gpr_strvec_destroy(&out); - - if (body_bytes) { - tmp = static_cast(gpr_realloc(tmp, out_len + body_size)); - memcpy(tmp + out_len, body_bytes, body_size); - out_len += body_size; + out.push_back("\r\n"); + std::string req = absl::StrJoin(out, ""); + if (body_bytes != nullptr) { + absl::StrAppend(&req, absl::string_view(body_bytes, body_size)); } - - return grpc_slice_new(tmp, out_len, gpr_free); + return grpc_slice_from_copied_buffer(req.data(), req.size()); } grpc_slice grpc_httpcli_format_connect_request( const grpc_httpcli_request* request) { - gpr_strvec out; - gpr_strvec_init(&out); - gpr_strvec_add(&out, gpr_strdup("CONNECT ")); - fill_common_header(request, &out, false); - gpr_strvec_add(&out, gpr_strdup("\r\n")); - size_t flat_len; - char* flat = gpr_strvec_flatten(&out, &flat_len); - gpr_strvec_destroy(&out); - return grpc_slice_new(flat, flat_len, gpr_free); + std::vector out; + out.push_back("CONNECT "); + fill_common_header(request, false, &out); + out.push_back("\r\n"); + std::string req = absl::StrJoin(out, ""); + return grpc_slice_from_copied_buffer(req.data(), req.size()); } diff --git a/src/core/lib/iomgr/ev_epoll1_linux.cc b/src/core/lib/iomgr/ev_epoll1_linux.cc index 392d38b5d17..04955e1505b 100644 --- a/src/core/lib/iomgr/ev_epoll1_linux.cc +++ b/src/core/lib/iomgr/ev_epoll1_linux.cc @@ -38,6 +38,11 @@ #include #include +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include @@ -1064,30 +1069,23 @@ static grpc_error* pollset_kick(grpc_pollset* pollset, GRPC_STATS_INC_POLLSET_KICK(); grpc_error* ret_err = GRPC_ERROR_NONE; if (GRPC_TRACE_FLAG_ENABLED(grpc_polling_trace)) { - gpr_strvec log; - gpr_strvec_init(&log); - char* tmp; - gpr_asprintf(&tmp, "PS:%p KICK:%p curps=%p curworker=%p root=%p", pollset, - specific_worker, (void*)gpr_tls_get(&g_current_thread_pollset), - (void*)gpr_tls_get(&g_current_thread_worker), - pollset->root_worker); - gpr_strvec_add(&log, tmp); + std::vector log; + log.push_back(absl::StrFormat( + "PS:%p KICK:%p curps=%p curworker=%p root=%p", pollset, specific_worker, + (void*)gpr_tls_get(&g_current_thread_pollset), + (void*)gpr_tls_get(&g_current_thread_worker), pollset->root_worker)); if (pollset->root_worker != nullptr) { - gpr_asprintf(&tmp, " {kick_state=%s next=%p {kick_state=%s}}", - kick_state_string(pollset->root_worker->state), - pollset->root_worker->next, - kick_state_string(pollset->root_worker->next->state)); - gpr_strvec_add(&log, tmp); + log.push_back(absl::StrFormat( + " {kick_state=%s next=%p {kick_state=%s}}", + kick_state_string(pollset->root_worker->state), + pollset->root_worker->next, + kick_state_string(pollset->root_worker->next->state))); } if (specific_worker != nullptr) { - gpr_asprintf(&tmp, " worker_kick_state=%s", - kick_state_string(specific_worker->state)); - gpr_strvec_add(&log, tmp); + log.push_back(absl::StrFormat(" worker_kick_state=%s", + kick_state_string(specific_worker->state))); } - tmp = gpr_strvec_flatten(&log, nullptr); - gpr_strvec_destroy(&log); - gpr_log(GPR_DEBUG, "%s", tmp); - gpr_free(tmp); + gpr_log(GPR_DEBUG, "%s", absl::StrJoin(log, "").c_str()); } if (specific_worker == nullptr) { diff --git a/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc b/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc index 8191043db21..550aa0cb544 100644 --- a/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc +++ b/src/core/lib/security/credentials/oauth2/oauth2_credentials.cc @@ -24,6 +24,7 @@ #include #include "absl/container/inlined_vector.h" +#include "absl/strings/str_join.h" #include #include @@ -523,12 +524,10 @@ namespace grpc_core { namespace { -void MaybeAddToBody(gpr_strvec* body_strvec, const char* field_name, - const char* field) { +void MaybeAddToBody(const char* field_name, const char* field, + std::vector* body) { if (field == nullptr || strlen(field) == 0) return; - char* new_query; - gpr_asprintf(&new_query, "&%s=%s", field_name, field); - gpr_strvec_add(body_strvec, new_query); + body->push_back(absl::StrFormat("&%s=%s", field_name, field)); } grpc_error* LoadTokenFile(const char* path, gpr_slice* token) { @@ -608,20 +607,18 @@ class StsTokenFetcherCredentials grpc_error* FillBody(char** body, size_t* body_length) { *body = nullptr; - gpr_strvec body_strvec; - gpr_strvec_init(&body_strvec); + std::vector body_parts; grpc_slice subject_token = grpc_empty_slice(); grpc_slice actor_token = grpc_empty_slice(); grpc_error* err = GRPC_ERROR_NONE; - auto cleanup = [&body, &body_length, &body_strvec, &subject_token, + auto cleanup = [&body, &body_length, &body_parts, &subject_token, &actor_token, &err]() { if (err == GRPC_ERROR_NONE) { - *body = gpr_strvec_flatten(&body_strvec, body_length); - } else { - gpr_free(*body); + std::string body_str = absl::StrJoin(body_parts, ""); + *body = gpr_strdup(body_str.c_str()); + *body_length = body_str.size(); } - gpr_strvec_destroy(&body_strvec); grpc_slice_unref_internal(subject_token); grpc_slice_unref_internal(actor_token); return err; @@ -629,23 +626,23 @@ class StsTokenFetcherCredentials err = LoadTokenFile(subject_token_path_.get(), &subject_token); if (err != GRPC_ERROR_NONE) return cleanup(); - gpr_asprintf( - body, GRPC_STS_POST_MINIMAL_BODY_FORMAT_STRING, + body_parts.push_back(absl::StrFormat( + GRPC_STS_POST_MINIMAL_BODY_FORMAT_STRING, reinterpret_cast(GRPC_SLICE_START_PTR(subject_token)), - subject_token_type_.get()); - gpr_strvec_add(&body_strvec, *body); - MaybeAddToBody(&body_strvec, "resource", resource_.get()); - MaybeAddToBody(&body_strvec, "audience", audience_.get()); - MaybeAddToBody(&body_strvec, "scope", scope_.get()); - MaybeAddToBody(&body_strvec, "requested_token_type", - requested_token_type_.get()); + subject_token_type_.get())); + MaybeAddToBody("resource", resource_.get(), &body_parts); + MaybeAddToBody("audience", audience_.get(), &body_parts); + MaybeAddToBody("scope", scope_.get(), &body_parts); + MaybeAddToBody("requested_token_type", requested_token_type_.get(), + &body_parts); if ((actor_token_path_ != nullptr) && *actor_token_path_ != '\0') { err = LoadTokenFile(actor_token_path_.get(), &actor_token); if (err != GRPC_ERROR_NONE) return cleanup(); MaybeAddToBody( - &body_strvec, "actor_token", - reinterpret_cast(GRPC_SLICE_START_PTR(actor_token))); - MaybeAddToBody(&body_strvec, "actor_token_type", actor_token_type_.get()); + "actor_token", + reinterpret_cast(GRPC_SLICE_START_PTR(actor_token)), + &body_parts); + MaybeAddToBody("actor_token_type", actor_token_type_.get(), &body_parts); } return cleanup(); } diff --git a/src/core/lib/surface/call_log_batch.cc b/src/core/lib/surface/call_log_batch.cc index 23c344a2c0e..eac55137183 100644 --- a/src/core/lib/surface/call_log_batch.cc +++ b/src/core/lib/surface/call_log_batch.cc @@ -22,98 +22,90 @@ #include +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include "src/core/lib/gpr/string.h" #include "src/core/lib/slice/slice_string_helpers.h" -static void add_metadata(gpr_strvec* b, const grpc_metadata* md, size_t count) { - size_t i; +static void add_metadata(const grpc_metadata* md, size_t count, + std::vector* b) { if (md == nullptr) { - gpr_strvec_add(b, gpr_strdup("(nil)")); + b->push_back("(nil)"); return; } - for (i = 0; i < count; i++) { - gpr_strvec_add(b, gpr_strdup("\nkey=")); - gpr_strvec_add(b, grpc_slice_to_c_string(md[i].key)); - - gpr_strvec_add(b, gpr_strdup(" value=")); - gpr_strvec_add(b, - grpc_dump_slice(md[i].value, GPR_DUMP_HEX | GPR_DUMP_ASCII)); + for (size_t i = 0; i < count; i++) { + b->push_back("\nkey="); + b->push_back(std::string(grpc_core::StringViewFromSlice(md[i].key))); + b->push_back(" value="); + char* dump = grpc_dump_slice(md[i].value, GPR_DUMP_HEX | GPR_DUMP_ASCII); + b->push_back(dump); + gpr_free(dump); } } -char* grpc_op_string(const grpc_op* op) { - char* tmp; - char* out; - - gpr_strvec b; - gpr_strvec_init(&b); - +static std::string grpc_op_string(const grpc_op* op) { + std::vector parts; switch (op->op) { case GRPC_OP_SEND_INITIAL_METADATA: - gpr_strvec_add(&b, gpr_strdup("SEND_INITIAL_METADATA")); - add_metadata(&b, op->data.send_initial_metadata.metadata, - op->data.send_initial_metadata.count); + parts.push_back("SEND_INITIAL_METADATA"); + add_metadata(op->data.send_initial_metadata.metadata, + op->data.send_initial_metadata.count, &parts); break; case GRPC_OP_SEND_MESSAGE: - gpr_asprintf(&tmp, "SEND_MESSAGE ptr=%p", - op->data.send_message.send_message); - gpr_strvec_add(&b, tmp); + parts.push_back(absl::StrFormat("SEND_MESSAGE ptr=%p", + op->data.send_message.send_message)); break; case GRPC_OP_SEND_CLOSE_FROM_CLIENT: - gpr_strvec_add(&b, gpr_strdup("SEND_CLOSE_FROM_CLIENT")); + parts.push_back("SEND_CLOSE_FROM_CLIENT"); break; case GRPC_OP_SEND_STATUS_FROM_SERVER: - gpr_asprintf(&tmp, "SEND_STATUS_FROM_SERVER status=%d details=", - op->data.send_status_from_server.status); - gpr_strvec_add(&b, tmp); + parts.push_back( + absl::StrFormat("SEND_STATUS_FROM_SERVER status=%d details=", + op->data.send_status_from_server.status)); if (op->data.send_status_from_server.status_details != nullptr) { - gpr_strvec_add(&b, grpc_dump_slice( - *op->data.send_status_from_server.status_details, - GPR_DUMP_ASCII)); + char* dump = grpc_dump_slice( + *op->data.send_status_from_server.status_details, GPR_DUMP_ASCII); + parts.push_back(dump); + gpr_free(dump); } else { - gpr_strvec_add(&b, gpr_strdup("(null)")); + parts.push_back("(null)"); } - add_metadata(&b, op->data.send_status_from_server.trailing_metadata, - op->data.send_status_from_server.trailing_metadata_count); + add_metadata(op->data.send_status_from_server.trailing_metadata, + op->data.send_status_from_server.trailing_metadata_count, + &parts); break; case GRPC_OP_RECV_INITIAL_METADATA: - gpr_asprintf(&tmp, "RECV_INITIAL_METADATA ptr=%p", - op->data.recv_initial_metadata.recv_initial_metadata); - gpr_strvec_add(&b, tmp); + parts.push_back(absl::StrFormat( + "RECV_INITIAL_METADATA ptr=%p", + op->data.recv_initial_metadata.recv_initial_metadata)); break; case GRPC_OP_RECV_MESSAGE: - gpr_asprintf(&tmp, "RECV_MESSAGE ptr=%p", - op->data.recv_message.recv_message); - gpr_strvec_add(&b, tmp); + parts.push_back(absl::StrFormat("RECV_MESSAGE ptr=%p", + op->data.recv_message.recv_message)); break; case GRPC_OP_RECV_STATUS_ON_CLIENT: - gpr_asprintf(&tmp, - "RECV_STATUS_ON_CLIENT metadata=%p status=%p details=%p", - op->data.recv_status_on_client.trailing_metadata, - op->data.recv_status_on_client.status, - op->data.recv_status_on_client.status_details); - gpr_strvec_add(&b, tmp); + parts.push_back(absl::StrFormat( + "RECV_STATUS_ON_CLIENT metadata=%p status=%p details=%p", + op->data.recv_status_on_client.trailing_metadata, + op->data.recv_status_on_client.status, + op->data.recv_status_on_client.status_details)); break; case GRPC_OP_RECV_CLOSE_ON_SERVER: - gpr_asprintf(&tmp, "RECV_CLOSE_ON_SERVER cancelled=%p", - op->data.recv_close_on_server.cancelled); - gpr_strvec_add(&b, tmp); + parts.push_back(absl::StrFormat("RECV_CLOSE_ON_SERVER cancelled=%p", + op->data.recv_close_on_server.cancelled)); } - out = gpr_strvec_flatten(&b, nullptr); - gpr_strvec_destroy(&b); - - return out; + return absl::StrJoin(parts, ""); } void grpc_call_log_batch(const char* file, int line, gpr_log_severity severity, const grpc_op* ops, size_t nops) { - char* tmp; - size_t i; - for (i = 0; i < nops; i++) { - tmp = grpc_op_string(&ops[i]); - gpr_log(file, line, severity, "ops[%" PRIuPTR "]: %s", i, tmp); - gpr_free(tmp); + for (size_t i = 0; i < nops; i++) { + gpr_log(file, line, severity, "ops[%" PRIuPTR "]: %s", i, + grpc_op_string(&ops[i]).c_str()); } } diff --git a/src/core/lib/surface/completion_queue.cc b/src/core/lib/surface/completion_queue.cc index 7c0c9aeb972..fa02c5628d2 100644 --- a/src/core/lib/surface/completion_queue.cc +++ b/src/core/lib/surface/completion_queue.cc @@ -23,6 +23,11 @@ #include #include +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include @@ -430,15 +435,14 @@ static const cq_vtable g_cq_vtable[] = { grpc_core::TraceFlag grpc_cq_pluck_trace(false, "queue_pluck"); -#define GRPC_SURFACE_TRACE_RETURNED_EVENT(cq, event) \ - do { \ - if (GRPC_TRACE_FLAG_ENABLED(grpc_api_trace) && \ - (GRPC_TRACE_FLAG_ENABLED(grpc_cq_pluck_trace) || \ - (event)->type != GRPC_QUEUE_TIMEOUT)) { \ - char* _ev = grpc_event_string(event); \ - gpr_log(GPR_INFO, "RETURN_EVENT[%p]: %s", cq, _ev); \ - gpr_free(_ev); \ - } \ +#define GRPC_SURFACE_TRACE_RETURNED_EVENT(cq, event) \ + do { \ + if (GRPC_TRACE_FLAG_ENABLED(grpc_api_trace) && \ + (GRPC_TRACE_FLAG_ENABLED(grpc_cq_pluck_trace) || \ + (event)->type != GRPC_QUEUE_TIMEOUT)) { \ + gpr_log(GPR_INFO, "RETURN_EVENT[%p]: %s", cq, \ + grpc_event_string(event).c_str()); \ + } \ } while (0) static void on_pollset_shutdown_done(void* cq, grpc_error* error); @@ -951,21 +955,14 @@ class ExecCtxNext : public grpc_core::ExecCtx { #ifndef NDEBUG static void dump_pending_tags(grpc_completion_queue* cq) { if (!GRPC_TRACE_FLAG_ENABLED(grpc_trace_pending_tags)) return; - - gpr_strvec v; - gpr_strvec_init(&v); - gpr_strvec_add(&v, gpr_strdup("PENDING TAGS:")); + std::vector parts; + parts.push_back("PENDING TAGS:"); gpr_mu_lock(cq->mu); for (size_t i = 0; i < cq->outstanding_tag_count; i++) { - char* s; - gpr_asprintf(&s, " %p", cq->outstanding_tags[i]); - gpr_strvec_add(&v, s); + parts.push_back(absl::StrFormat(" %p", cq->outstanding_tags[i])); } gpr_mu_unlock(cq->mu); - char* out = gpr_strvec_flatten(&v, nullptr); - gpr_strvec_destroy(&v); - gpr_log(GPR_DEBUG, "%s", out); - gpr_free(out); + gpr_log(GPR_DEBUG, "%s", absl::StrJoin(parts, "").c_str()); } #else static void dump_pending_tags(grpc_completion_queue* /*cq*/) {} diff --git a/src/core/lib/surface/event_string.cc b/src/core/lib/surface/event_string.cc index d639baec455..32d85f7b054 100644 --- a/src/core/lib/surface/event_string.cc +++ b/src/core/lib/surface/event_string.cc @@ -22,47 +22,40 @@ #include +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include "src/core/lib/gpr/string.h" -static void addhdr(gpr_strvec* buf, grpc_event* ev) { - char* tmp; - gpr_asprintf(&tmp, "tag:%p", ev->tag); - gpr_strvec_add(buf, tmp); +static void addhdr(grpc_event* ev, std::vector* buf) { + buf->push_back(absl::StrFormat("tag:%p", ev->tag)); } static const char* errstr(int success) { return success ? "OK" : "ERROR"; } -static void adderr(gpr_strvec* buf, int success) { - char* tmp; - gpr_asprintf(&tmp, " %s", errstr(success)); - gpr_strvec_add(buf, tmp); +static void adderr(int success, std::vector* buf) { + buf->push_back(absl::StrFormat(" %s", errstr(success))); } -char* grpc_event_string(grpc_event* ev) { - char* out; - gpr_strvec buf; - - if (ev == nullptr) return gpr_strdup("null"); - - gpr_strvec_init(&buf); - +std::string grpc_event_string(grpc_event* ev) { + if (ev == nullptr) return "null"; + std::vector out; switch (ev->type) { case GRPC_QUEUE_TIMEOUT: - gpr_strvec_add(&buf, gpr_strdup("QUEUE_TIMEOUT")); + out.push_back("QUEUE_TIMEOUT"); break; case GRPC_QUEUE_SHUTDOWN: - gpr_strvec_add(&buf, gpr_strdup("QUEUE_SHUTDOWN")); + out.push_back("QUEUE_SHUTDOWN"); break; case GRPC_OP_COMPLETE: - gpr_strvec_add(&buf, gpr_strdup("OP_COMPLETE: ")); - addhdr(&buf, ev); - adderr(&buf, ev->success); + out.push_back("OP_COMPLETE: "); + addhdr(ev, &out); + adderr(ev->success, &out); break; } - - out = gpr_strvec_flatten(&buf, nullptr); - gpr_strvec_destroy(&buf); - return out; + return absl::StrJoin(out, ""); } diff --git a/src/core/lib/surface/event_string.h b/src/core/lib/surface/event_string.h index e6095705e9b..9e02a52e5cb 100644 --- a/src/core/lib/surface/event_string.h +++ b/src/core/lib/surface/event_string.h @@ -21,9 +21,11 @@ #include +#include + #include /* Returns a string describing an event. Must be later freed with gpr_free() */ -char* grpc_event_string(grpc_event* ev); +std::string grpc_event_string(grpc_event* ev); #endif /* GRPC_CORE_LIB_SURFACE_EVENT_STRING_H */ diff --git a/src/core/lib/transport/transport.h b/src/core/lib/transport/transport.h index 18374511cde..ab512896404 100644 --- a/src/core/lib/transport/transport.h +++ b/src/core/lib/transport/transport.h @@ -408,8 +408,9 @@ void grpc_transport_stream_op_batch_finish_with_failure( grpc_transport_stream_op_batch* op, grpc_error* error, grpc_core::CallCombiner* call_combiner); -char* grpc_transport_stream_op_batch_string(grpc_transport_stream_op_batch* op); -char* grpc_transport_op_string(grpc_transport_op* op); +std::string grpc_transport_stream_op_batch_string( + grpc_transport_stream_op_batch* op); +std::string grpc_transport_op_string(grpc_transport_op* op); /* Send a batch of operations on a transport diff --git a/src/core/lib/transport/transport_op_string.cc b/src/core/lib/transport/transport_op_string.cc index 34b36c3aec9..722a64ecb8d 100644 --- a/src/core/lib/transport/transport_op_string.cc +++ b/src/core/lib/transport/transport_op_string.cc @@ -25,6 +25,12 @@ #include #include +#include + +#include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include "src/core/lib/gpr/string.h" @@ -34,177 +40,130 @@ /* These routines are here to facilitate debugging - they produce string representations of various transport data structures */ -static void put_metadata(gpr_strvec* b, grpc_mdelem md) { - gpr_strvec_add(b, gpr_strdup("key=")); - gpr_strvec_add( - b, grpc_dump_slice(GRPC_MDKEY(md), GPR_DUMP_HEX | GPR_DUMP_ASCII)); - - gpr_strvec_add(b, gpr_strdup(" value=")); - gpr_strvec_add( - b, grpc_dump_slice(GRPC_MDVALUE(md), GPR_DUMP_HEX | GPR_DUMP_ASCII)); +static void put_metadata(grpc_mdelem md, std::vector* out) { + out->push_back("key="); + char* dump = grpc_dump_slice(GRPC_MDKEY(md), GPR_DUMP_HEX | GPR_DUMP_ASCII); + out->push_back(dump); + gpr_free(dump); + out->push_back(" value="); + dump = grpc_dump_slice(GRPC_MDVALUE(md), GPR_DUMP_HEX | GPR_DUMP_ASCII); + out->push_back(dump); + gpr_free(dump); } -static void put_metadata_list(gpr_strvec* b, grpc_metadata_batch md) { +static void put_metadata_list(grpc_metadata_batch md, + std::vector* out) { grpc_linked_mdelem* m; for (m = md.list.head; m != nullptr; m = m->next) { - if (m != md.list.head) gpr_strvec_add(b, gpr_strdup(", ")); - put_metadata(b, m->md); + if (m != md.list.head) out->push_back(", "); + put_metadata(m->md, out); } if (md.deadline != GRPC_MILLIS_INF_FUTURE) { - char* tmp; - gpr_asprintf(&tmp, " deadline=%" PRId64, md.deadline); - gpr_strvec_add(b, tmp); + out->push_back(absl::StrFormat(" deadline=%" PRId64, md.deadline)); } } -char* grpc_transport_stream_op_batch_string( +std::string grpc_transport_stream_op_batch_string( grpc_transport_stream_op_batch* op) { - char* tmp; - char* out; - - gpr_strvec b; - gpr_strvec_init(&b); + std::vector out; if (op->send_initial_metadata) { - gpr_strvec_add(&b, gpr_strdup(" ")); - gpr_strvec_add(&b, gpr_strdup("SEND_INITIAL_METADATA{")); - put_metadata_list( - &b, *op->payload->send_initial_metadata.send_initial_metadata); - gpr_strvec_add(&b, gpr_strdup("}")); + out.push_back(" SEND_INITIAL_METADATA{"); + put_metadata_list(*op->payload->send_initial_metadata.send_initial_metadata, + &out); + out.push_back("}"); } if (op->send_message) { - gpr_strvec_add(&b, gpr_strdup(" ")); if (op->payload->send_message.send_message != nullptr) { - gpr_asprintf(&tmp, "SEND_MESSAGE:flags=0x%08x:len=%d", - op->payload->send_message.send_message->flags(), - op->payload->send_message.send_message->length()); + out.push_back( + absl::StrFormat(" SEND_MESSAGE:flags=0x%08x:len=%d", + op->payload->send_message.send_message->flags(), + op->payload->send_message.send_message->length())); } else { // This can happen when we check a batch after the transport has // processed and cleared the send_message op. - tmp = - gpr_strdup("SEND_MESSAGE(flag and length unknown, already orphaned)"); + out.push_back(" SEND_MESSAGE(flag and length unknown, already orphaned)"); } - gpr_strvec_add(&b, tmp); } if (op->send_trailing_metadata) { - gpr_strvec_add(&b, gpr_strdup(" ")); - gpr_strvec_add(&b, gpr_strdup("SEND_TRAILING_METADATA{")); + out.push_back(" SEND_TRAILING_METADATA{"); put_metadata_list( - &b, *op->payload->send_trailing_metadata.send_trailing_metadata); - gpr_strvec_add(&b, gpr_strdup("}")); + *op->payload->send_trailing_metadata.send_trailing_metadata, &out); + out.push_back("}"); } if (op->recv_initial_metadata) { - gpr_strvec_add(&b, gpr_strdup(" ")); - gpr_strvec_add(&b, gpr_strdup("RECV_INITIAL_METADATA")); + out.push_back(" RECV_INITIAL_METADATA"); } if (op->recv_message) { - gpr_strvec_add(&b, gpr_strdup(" ")); - gpr_strvec_add(&b, gpr_strdup("RECV_MESSAGE")); + out.push_back(" RECV_MESSAGE"); } if (op->recv_trailing_metadata) { - gpr_strvec_add(&b, gpr_strdup(" ")); - gpr_strvec_add(&b, gpr_strdup("RECV_TRAILING_METADATA")); + out.push_back(" RECV_TRAILING_METADATA"); } if (op->cancel_stream) { - gpr_strvec_add(&b, gpr_strdup(" ")); - const char* msg = - grpc_error_string(op->payload->cancel_stream.cancel_error); - gpr_asprintf(&tmp, "CANCEL:%s", msg); - - gpr_strvec_add(&b, tmp); + out.push_back(absl::StrCat( + " CANCEL:", + grpc_error_string(op->payload->cancel_stream.cancel_error))); } - out = gpr_strvec_flatten(&b, nullptr); - gpr_strvec_destroy(&b); - - return out; + return absl::StrJoin(out, ""); } -char* grpc_transport_op_string(grpc_transport_op* op) { - char* tmp; - char* out; - bool first = true; - - gpr_strvec b; - gpr_strvec_init(&b); +std::string grpc_transport_op_string(grpc_transport_op* op) { + std::vector out; if (op->start_connectivity_watch != nullptr) { - if (!first) gpr_strvec_add(&b, gpr_strdup(" ")); - first = false; - gpr_asprintf( - &tmp, "START_CONNECTIVITY_WATCH:watcher=%p:from=%s", + out.push_back(absl::StrFormat( + " START_CONNECTIVITY_WATCH:watcher=%p:from=%s", op->start_connectivity_watch.get(), - grpc_core::ConnectivityStateName(op->start_connectivity_watch_state)); - gpr_strvec_add(&b, tmp); + grpc_core::ConnectivityStateName(op->start_connectivity_watch_state))); } if (op->stop_connectivity_watch != nullptr) { - if (!first) gpr_strvec_add(&b, gpr_strdup(" ")); - first = false; - gpr_asprintf(&tmp, "STOP_CONNECTIVITY_WATCH:watcher=%p", - op->stop_connectivity_watch); - gpr_strvec_add(&b, tmp); + out.push_back(absl::StrFormat(" STOP_CONNECTIVITY_WATCH:watcher=%p", + op->stop_connectivity_watch)); } if (op->disconnect_with_error != GRPC_ERROR_NONE) { - if (!first) gpr_strvec_add(&b, gpr_strdup(" ")); - first = false; - const char* err = grpc_error_string(op->disconnect_with_error); - gpr_asprintf(&tmp, "DISCONNECT:%s", err); - gpr_strvec_add(&b, tmp); + out.push_back(absl::StrCat(" DISCONNECT:", + grpc_error_string(op->disconnect_with_error))); } if (op->goaway_error) { - if (!first) gpr_strvec_add(&b, gpr_strdup(" ")); - first = false; - const char* msg = grpc_error_string(op->goaway_error); - gpr_asprintf(&tmp, "SEND_GOAWAY:%s", msg); - - gpr_strvec_add(&b, tmp); + out.push_back( + absl::StrCat(" SEND_GOAWAY:%s", grpc_error_string(op->goaway_error))); } if (op->set_accept_stream) { - if (!first) gpr_strvec_add(&b, gpr_strdup(" ")); - first = false; - gpr_asprintf(&tmp, "SET_ACCEPT_STREAM:%p(%p,...)", op->set_accept_stream_fn, - op->set_accept_stream_user_data); - gpr_strvec_add(&b, tmp); + out.push_back(absl::StrFormat(" SET_ACCEPT_STREAM:%p(%p,...)", + op->set_accept_stream_fn, + op->set_accept_stream_user_data)); } if (op->bind_pollset != nullptr) { - if (!first) gpr_strvec_add(&b, gpr_strdup(" ")); - first = false; - gpr_strvec_add(&b, gpr_strdup("BIND_POLLSET")); + out.push_back(" BIND_POLLSET"); } if (op->bind_pollset_set != nullptr) { - if (!first) gpr_strvec_add(&b, gpr_strdup(" ")); - first = false; - gpr_strvec_add(&b, gpr_strdup("BIND_POLLSET_SET")); + out.push_back(" BIND_POLLSET_SET"); } if (op->send_ping.on_initiate != nullptr || op->send_ping.on_ack != nullptr) { - if (!first) gpr_strvec_add(&b, gpr_strdup(" ")); - // first = false; - gpr_strvec_add(&b, gpr_strdup("SEND_PING")); + out.push_back(" SEND_PING"); } - out = gpr_strvec_flatten(&b, nullptr); - gpr_strvec_destroy(&b); - - return out; + return absl::StrJoin(out, ""); } void grpc_call_log_op(const char* file, int line, gpr_log_severity severity, grpc_call_element* elem, grpc_transport_stream_op_batch* op) { - char* str = grpc_transport_stream_op_batch_string(op); - gpr_log(file, line, severity, "OP[%s:%p]: %s", elem->filter->name, elem, str); - gpr_free(str); + gpr_log(file, line, severity, "OP[%s:%p]: %s", elem->filter->name, elem, + grpc_transport_stream_op_batch_string(op).c_str()); } diff --git a/src/python/grpcio/grpc/_cython/_cygrpc/iomgr.pxd.pxi b/src/python/grpcio/grpc/_cython/_cygrpc/iomgr.pxd.pxi index 3b64b284aa9..0c5a4e5763d 100644 --- a/src/python/grpcio/grpc/_cython/_cygrpc/iomgr.pxd.pxi +++ b/src/python/grpcio/grpc/_cython/_cygrpc/iomgr.pxd.pxi @@ -122,7 +122,7 @@ cdef extern from "src/core/lib/iomgr/iomgr_custom.h": 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); + bool_t normalize); void 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) diff --git a/test/core/bad_client/tests/large_metadata.cc b/test/core/bad_client/tests/large_metadata.cc index 8206afa5afb..e8b153f4dd9 100644 --- a/test/core/bad_client/tests/large_metadata.cc +++ b/test/core/bad_client/tests/large_metadata.cc @@ -20,6 +20,9 @@ #include +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include "src/core/lib/gpr/string.h" @@ -144,22 +147,17 @@ int main(int argc, char** argv) { grpc::testing::TestEnvironment env(argc, argv); // Test sending more metadata than the server will accept. - gpr_strvec headers; - gpr_strvec_init(&headers); + std::vector headers; for (i = 0; i < NUM_HEADERS; ++i) { - char* str; - gpr_asprintf(&str, "%s%02d%s", - PFX_TOO_MUCH_METADATA_FROM_CLIENT_HEADER_START_STR, i, - PFX_TOO_MUCH_METADATA_FROM_CLIENT_HEADER_END_STR); - gpr_strvec_add(&headers, str); + headers.push_back(absl::StrFormat( + "%s%02d%s", PFX_TOO_MUCH_METADATA_FROM_CLIENT_HEADER_START_STR, i, + PFX_TOO_MUCH_METADATA_FROM_CLIENT_HEADER_END_STR)); } - size_t headers_len; - const char* client_headers = gpr_strvec_flatten(&headers, &headers_len); - gpr_strvec_destroy(&headers); + std::string client_headers = absl::StrJoin(headers, ""); char client_payload[TOO_MUCH_METADATA_FROM_CLIENT_REQUEST_SIZE] = PFX_TOO_MUCH_METADATA_FROM_CLIENT_REQUEST; memcpy(client_payload + sizeof(PFX_TOO_MUCH_METADATA_FROM_CLIENT_REQUEST) - 1, - client_headers, headers_len); + client_headers.data(), client_headers.size()); grpc_bad_client_arg args[2]; args[0] = connection_preface_arg; args[1].client_validator = rst_stream_client_validator; @@ -167,7 +165,6 @@ int main(int argc, char** argv) { args[1].client_payload_length = sizeof(client_payload) - 1; grpc_run_bad_client_test(server_verifier_request_call, args, 2, 0); - gpr_free((void*)client_headers); // Test sending more metadata than the client will accept. GRPC_RUN_BAD_CLIENT_TEST(server_verifier_sends_too_much_metadata, diff --git a/test/core/channel/minimal_stack_is_minimal_test.cc b/test/core/channel/minimal_stack_is_minimal_test.cc index 858fd5676a1..455d6ae3634 100644 --- a/test/core/channel/minimal_stack_is_minimal_test.cc +++ b/test/core/channel/minimal_stack_is_minimal_test.cc @@ -34,6 +34,10 @@ #include #include +#include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include "src/core/lib/channel/channel_stack_builder.h" #include "src/core/lib/gpr/string.h" #include "src/core/lib/surface/channel_init.h" @@ -138,64 +142,51 @@ static int check_stack(const char* file, int line, const char* transport_name, } // build up our expectation list - gpr_strvec v; - gpr_strvec_init(&v); + std::vector parts; va_list args; va_start(args, channel_stack_type); for (;;) { char* a = va_arg(args, char*); if (a == nullptr) break; - if (v.count != 0) gpr_strvec_add(&v, gpr_strdup(", ")); - gpr_strvec_add(&v, gpr_strdup(a)); + parts.push_back(a); } va_end(args); - char* expect = gpr_strvec_flatten(&v, nullptr); - gpr_strvec_destroy(&v); + std::string expect = absl::StrJoin(parts, ", "); // build up our "got" list - gpr_strvec_init(&v); + parts.clear(); grpc_channel_stack_builder_iterator* it = grpc_channel_stack_builder_create_iterator_at_first(builder); while (grpc_channel_stack_builder_move_next(it)) { const char* name = grpc_channel_stack_builder_iterator_filter_name(it); if (name == nullptr) continue; - if (v.count != 0) gpr_strvec_add(&v, gpr_strdup(", ")); - gpr_strvec_add(&v, gpr_strdup(name)); + parts.push_back(name); } - char* got = gpr_strvec_flatten(&v, nullptr); - gpr_strvec_destroy(&v); + std::string got = absl::StrJoin(parts, ", "); grpc_channel_stack_builder_iterator_destroy(it); // figure out result, log if there's an error int result = 0; - if (0 != strcmp(got, expect)) { - gpr_strvec_init(&v); - gpr_strvec_add(&v, gpr_strdup("{")); + if (got != expect) { + parts.clear(); for (size_t i = 0; i < channel_args->num_args; i++) { - if (i > 0) gpr_strvec_add(&v, gpr_strdup(", ")); - gpr_strvec_add(&v, gpr_strdup(channel_args->args[i].key)); - gpr_strvec_add(&v, gpr_strdup("=")); + std::string value; switch (channel_args->args[i].type) { case GRPC_ARG_INTEGER: { - char* tmp; - gpr_asprintf(&tmp, "%d", channel_args->args[i].value.integer); - gpr_strvec_add(&v, tmp); + value = absl::StrCat(channel_args->args[i].value.integer); break; } case GRPC_ARG_STRING: - gpr_strvec_add(&v, gpr_strdup(channel_args->args[i].value.string)); + value = channel_args->args[i].value.string; break; case GRPC_ARG_POINTER: { - char* tmp; - gpr_asprintf(&tmp, "%p", channel_args->args[i].value.pointer.p); - gpr_strvec_add(&v, tmp); + value = absl::StrFormat("%p", channel_args->args[i].value.pointer.p); break; } } + parts.push_back(absl::StrCat(channel_args->args[i].key, "=", value)); } - gpr_strvec_add(&v, gpr_strdup("}")); - char* args_str = gpr_strvec_flatten(&v, nullptr); - gpr_strvec_destroy(&v); + std::string args_str = absl::StrCat("{", absl::StrJoin(parts, ", "), "}"); gpr_log(file, line, GPR_LOG_SEVERITY_ERROR, "**************************************************"); @@ -204,17 +195,12 @@ static int check_stack(const char* file, int line, const char* transport_name, "FAILED transport=%s; stack_type=%s; channel_args=%s:", transport_name, grpc_channel_stack_type_string( static_cast(channel_stack_type)), - args_str); - gpr_log(file, line, GPR_LOG_SEVERITY_ERROR, "EXPECTED: %s", expect); - gpr_log(file, line, GPR_LOG_SEVERITY_ERROR, "GOT: %s", got); + args_str.c_str()); + gpr_log(file, line, GPR_LOG_SEVERITY_ERROR, "EXPECTED: %s", expect.c_str()); + gpr_log(file, line, GPR_LOG_SEVERITY_ERROR, "GOT: %s", got.c_str()); result = 1; - - gpr_free(args_str); } - gpr_free(got); - gpr_free(expect); - { grpc_core::ExecCtx exec_ctx; grpc_channel_stack_builder_destroy(builder); diff --git a/test/core/end2end/cq_verifier.cc b/test/core/end2end/cq_verifier.cc index f4e7fe29cf1..6030b496226 100644 --- a/test/core/end2end/cq_verifier.cc +++ b/test/core/end2end/cq_verifier.cc @@ -23,6 +23,12 @@ #include #include +#include +#include + +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include @@ -171,21 +177,19 @@ int byte_buffer_eq_string(grpc_byte_buffer* bb, const char* str) { static bool is_probably_integer(void* p) { return ((uintptr_t)p) < 1000000; } -static void expectation_to_strvec(gpr_strvec* buf, expectation* e) { - char* tmp; +namespace { - if (is_probably_integer(e->tag)) { - gpr_asprintf(&tmp, "tag(%" PRIdPTR ") ", (intptr_t)e->tag); +std::string ExpectationString(const expectation& e) { + std::string out; + if (is_probably_integer(e.tag)) { + out = absl::StrFormat("tag(%" PRIdPTR ") ", (intptr_t)e.tag); } else { - gpr_asprintf(&tmp, "%p ", e->tag); + out = absl::StrFormat("%p ", e.tag); } - gpr_strvec_add(buf, tmp); - - switch (e->type) { + switch (e.type) { case GRPC_OP_COMPLETE: - gpr_asprintf(&tmp, "GRPC_OP_COMPLETE success=%d %s:%d", e->success, - e->file, e->line); - gpr_strvec_add(buf, tmp); + absl::StrAppendFormat(&out, "GRPC_OP_COMPLETE success=%d %s:%d", + e.success, e.file, e.line); break; case GRPC_QUEUE_TIMEOUT: case GRPC_QUEUE_SHUTDOWN: @@ -193,27 +197,22 @@ static void expectation_to_strvec(gpr_strvec* buf, expectation* e) { abort(); break; } + return out; } -static void expectations_to_strvec(gpr_strvec* buf, cq_verifier* v) { - expectation* e; - - for (e = v->first_expectation; e != nullptr; e = e->next) { - expectation_to_strvec(buf, e); - gpr_strvec_add(buf, gpr_strdup("\n")); +std::string ExpectationsString(const cq_verifier& v) { + std::vector expectations; + for (expectation* e = v.first_expectation; e != nullptr; e = e->next) { + expectations.push_back(ExpectationString(*e)); } + return absl::StrJoin(expectations, "\n"); } +} // namespace + static void fail_no_event_received(cq_verifier* v) { - gpr_strvec buf; - char* msg; - gpr_strvec_init(&buf); - gpr_strvec_add(&buf, gpr_strdup("no event received, but expected:\n")); - expectations_to_strvec(&buf, v); - msg = gpr_strvec_flatten(&buf, nullptr); - gpr_log(GPR_ERROR, "%s", msg); - gpr_strvec_destroy(&buf); - gpr_free(msg); + gpr_log(GPR_ERROR, "no event received, but expected:%s", + ExpectationsString(*v).c_str()); abort(); } @@ -222,13 +221,8 @@ static void verify_matches(expectation* e, grpc_event* ev) { switch (e->type) { case GRPC_OP_COMPLETE: if (e->check_success && e->success != ev->success) { - gpr_strvec expected; - gpr_strvec_init(&expected); - expectation_to_strvec(&expected, e); - char* s = gpr_strvec_flatten(&expected, nullptr); - gpr_strvec_destroy(&expected); - gpr_log(GPR_ERROR, "actual success does not match expected: %s", s); - gpr_free(s); + gpr_log(GPR_ERROR, "actual success does not match expected: %s", + ExpectationString(*e).c_str()); abort(); } break; @@ -264,16 +258,9 @@ void cq_verify(cq_verifier* v) { prev = e; } if (e == nullptr) { - char* s = grpc_event_string(&ev); - gpr_log(GPR_ERROR, "cq returned unexpected event: %s", s); - gpr_free(s); - gpr_strvec expectations; - gpr_strvec_init(&expectations); - expectations_to_strvec(&expectations, v); - s = gpr_strvec_flatten(&expectations, nullptr); - gpr_strvec_destroy(&expectations); - gpr_log(GPR_ERROR, "expected tags:\n%s", s); - gpr_free(s); + gpr_log(GPR_ERROR, "cq returned unexpected event: %s", + grpc_event_string(&ev).c_str()); + gpr_log(GPR_ERROR, "expected tags:\n%s", ExpectationsString(*v).c_str()); abort(); } } @@ -290,9 +277,8 @@ void cq_verify_empty_timeout(cq_verifier* v, int timeout_sec) { ev = grpc_completion_queue_next(v->cq, deadline, nullptr); if (ev.type != GRPC_QUEUE_TIMEOUT) { - char* s = grpc_event_string(&ev); - gpr_log(GPR_ERROR, "unexpected event (expected nothing): %s", s); - gpr_free(s); + gpr_log(GPR_ERROR, "unexpected event (expected nothing): %s", + grpc_event_string(&ev).c_str()); abort(); } } diff --git a/test/core/end2end/tests/simple_request.cc b/test/core/end2end/tests/simple_request.cc index a8c57899e2c..8e0f863d847 100644 --- a/test/core/end2end/tests/simple_request.cc +++ b/test/core/end2end/tests/simple_request.cc @@ -21,6 +21,8 @@ #include #include +#include + #include #include #include @@ -245,9 +247,7 @@ static void simple_request_body(grpc_end2end_test_config config, grpc_stats_collect(after); - char* stats = grpc_stats_data_as_json(after); - gpr_log(GPR_DEBUG, "%s", stats); - gpr_free(stats); + gpr_log(GPR_DEBUG, "%s", grpc_stats_data_as_json(after).c_str()); GPR_ASSERT(after->counters[GRPC_STATS_COUNTER_CLIENT_CALLS_CREATED] - before->counters[GRPC_STATS_COUNTER_CLIENT_CALLS_CREATED] == diff --git a/test/core/gpr/arena_test.cc b/test/core/gpr/arena_test.cc index 1df1052ab95..c7aa63ab2e9 100644 --- a/test/core/gpr/arena_test.cc +++ b/test/core/gpr/arena_test.cc @@ -21,6 +21,9 @@ #include #include +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include @@ -37,20 +40,15 @@ static void test_noop(void) { Arena::Create(1)->Destroy(); } static void test(const char* name, size_t init_size, const size_t* allocs, size_t nallocs) { - gpr_strvec v; - char* s; - gpr_strvec_init(&v); - gpr_asprintf(&s, "test '%s': %" PRIdPTR " <- {", name, init_size); - gpr_strvec_add(&v, s); + std::vector parts; + parts.push_back( + absl::StrFormat("test '%s': %" PRIdPTR " <- {", name, init_size)); for (size_t i = 0; i < nallocs; i++) { - gpr_asprintf(&s, "%" PRIdPTR ",", allocs[i]); - gpr_strvec_add(&v, s); + parts.push_back(absl::StrFormat("%" PRIdPTR ",", allocs[i])); } - gpr_strvec_add(&v, gpr_strdup("}")); - s = gpr_strvec_flatten(&v, nullptr); - gpr_strvec_destroy(&v); - gpr_log(GPR_INFO, "%s", s); - gpr_free(s); + parts.push_back("}"); + std::string s = absl::StrJoin(parts, ""); + gpr_log(GPR_INFO, "%s", s.c_str()); Arena* a = Arena::Create(init_size); void** ps = static_cast(gpr_zalloc(sizeof(*ps) * nallocs)); diff --git a/test/core/security/verify_jwt.cc b/test/core/security/verify_jwt.cc index cfe285f53e6..2b480afcaa8 100644 --- a/test/core/security/verify_jwt.cc +++ b/test/core/security/verify_jwt.cc @@ -37,10 +37,9 @@ typedef struct { } synchronizer; static void print_usage_and_exit(gpr_cmdline* cl, const char* argv0) { - char* usage = gpr_cmdline_usage_string(cl, argv0); - fprintf(stderr, "%s", usage); + std::string usage = gpr_cmdline_usage_string(cl, argv0); + fprintf(stderr, "%s", usage.c_str()); fflush(stderr); - gpr_free(usage); gpr_cmdline_destroy(cl); exit(1); } diff --git a/test/core/util/cmdline.cc b/test/core/util/cmdline.cc index e76acb5e2b5..c213f84a73d 100644 --- a/test/core/util/cmdline.cc +++ b/test/core/util/cmdline.cc @@ -22,6 +22,12 @@ #include #include +#include + +#include "absl/strings/str_cat.h" +#include "absl/strings/str_format.h" +#include "absl/strings/str_join.h" + #include #include #include @@ -124,60 +130,42 @@ void gpr_cmdline_on_extra_arg( /* recursively descend argument list, adding the last element to s first - so that arguments are added in the order they were added to the list by api calls */ -static void add_args_to_usage(gpr_strvec* s, arg* a) { - char* tmp; - - if (!a) return; - add_args_to_usage(s, a->next); - +static void add_args_to_usage(arg* a, std::vector* s) { + if (a == nullptr) return; + add_args_to_usage(a->next, s); switch (a->type) { case ARGTYPE_BOOL: - gpr_asprintf(&tmp, " [--%s|--no-%s]", a->name, a->name); - gpr_strvec_add(s, tmp); + s->push_back(absl::StrFormat(" [--%s|--no-%s]", a->name, a->name)); break; case ARGTYPE_STRING: - gpr_asprintf(&tmp, " [--%s=string]", a->name); - gpr_strvec_add(s, tmp); + s->push_back(absl::StrFormat(" [--%s=string]", a->name)); break; case ARGTYPE_INT: - gpr_asprintf(&tmp, " [--%s=int]", a->name); - gpr_strvec_add(s, tmp); + s->push_back(absl::StrFormat(" [--%s=int]", a->name)); break; } } -char* gpr_cmdline_usage_string(gpr_cmdline* cl, const char* argv0) { - /* TODO(ctiller): make this prettier */ - gpr_strvec s; - char* tmp; +std::string gpr_cmdline_usage_string(gpr_cmdline* cl, const char* argv0) { const char* name = strrchr(argv0, '/'); - - if (name) { + if (name != nullptr) { name++; } else { name = argv0; } - gpr_strvec_init(&s); - - gpr_asprintf(&tmp, "Usage: %s", name); - gpr_strvec_add(&s, tmp); - add_args_to_usage(&s, cl->args); + std::vector s; + s.push_back(absl::StrCat("Usage: ", name)); + add_args_to_usage(cl->args, &s); if (cl->extra_arg) { - gpr_asprintf(&tmp, " [%s...]", cl->extra_arg_name); - gpr_strvec_add(&s, tmp); + s.push_back(absl::StrFormat(" [%s...]", cl->extra_arg_name)); } - gpr_strvec_add(&s, gpr_strdup("\n")); - - tmp = gpr_strvec_flatten(&s, nullptr); - gpr_strvec_destroy(&s); - return tmp; + s.push_back("\n"); + return absl::StrJoin(s, ""); } static int print_usage_and_die(gpr_cmdline* cl) { - char* usage = gpr_cmdline_usage_string(cl, cl->argv0); - fprintf(stderr, "%s", usage); - gpr_free(usage); + fprintf(stderr, "%s", gpr_cmdline_usage_string(cl, cl->argv0).c_str()); if (!cl->survive_failure) { exit(1); } diff --git a/test/core/util/cmdline.h b/test/core/util/cmdline.h index 3ae35d6e6af..773563dcbe9 100644 --- a/test/core/util/cmdline.h +++ b/test/core/util/cmdline.h @@ -21,6 +21,8 @@ #include +#include + /** Simple command line parser. Supports flags that can be specified as -foo, --foo, --no-foo, -no-foo, etc @@ -75,6 +77,6 @@ int gpr_cmdline_parse(gpr_cmdline* cl, int argc, char** argv); /** Destroy the parser */ void gpr_cmdline_destroy(gpr_cmdline* cl); /** Get a string describing usage */ -char* gpr_cmdline_usage_string(gpr_cmdline* cl, const char* argv0); +std::string gpr_cmdline_usage_string(gpr_cmdline* cl, const char* argv0); #endif /* GRPC_TEST_CORE_UTIL_CMDLINE_H */ diff --git a/test/core/util/cmdline_test.cc b/test/core/util/cmdline_test.cc index 59e1bbff34e..c15e5e36df9 100644 --- a/test/core/util/cmdline_test.cc +++ b/test/core/util/cmdline_test.cc @@ -307,7 +307,6 @@ static void test_extra_dashdash(void) { static void test_usage(void) { gpr_cmdline* cl; - char* usage; const char* str = nullptr; int x = 0; @@ -322,17 +321,15 @@ static void test_usage(void) { gpr_cmdline_on_extra_arg(cl, "file", "filenames to process", extra_arg_cb, nullptr); - usage = gpr_cmdline_usage_string(cl, "test"); - GPR_ASSERT(0 == strcmp(usage, - "Usage: test [--str=string] [--x=int] " - "[--flag|--no-flag] [file...]\n")); - gpr_free(usage); + std::string usage = gpr_cmdline_usage_string(cl, "test"); + GPR_ASSERT(usage == + "Usage: test [--str=string] [--x=int] " + "[--flag|--no-flag] [file...]\n"); usage = gpr_cmdline_usage_string(cl, "/foo/test"); - GPR_ASSERT(0 == strcmp(usage, - "Usage: test [--str=string] [--x=int] " - "[--flag|--no-flag] [file...]\n")); - gpr_free(usage); + GPR_ASSERT(usage == + "Usage: test [--str=string] [--x=int] " + "[--flag|--no-flag] [file...]\n"); gpr_cmdline_destroy(cl); }