From de81f120b20193496ffd28a02bf028b007b196a0 Mon Sep 17 00:00:00 2001 From: Esun Kim Date: Mon, 4 Nov 2019 15:55:18 -0800 Subject: [PATCH] Abseilify StringView --- bazel/grpc_build_system.bzl | 5 ++ .../filters/client_channel/client_channel.cc | 5 +- .../filters/client_channel/xds/xds_client.cc | 4 +- src/core/lib/gprpp/host_port.cc | 10 +-- src/core/lib/gprpp/string_view.h | 62 +++++++++++-------- .../ssl/ssl_security_connector.cc | 2 +- .../security/security_connector/ssl_utils.cc | 5 +- .../tls/spiffe_security_connector.cc | 2 +- .../security/transport/client_auth_filter.cc | 3 +- src/core/tsi/ssl_transport_security.cc | 3 +- test/core/gprpp/string_view_test.cc | 35 ++++++----- test/core/security/credentials_test.cc | 4 +- test/cpp/end2end/client_lb_end2end_test.cc | 6 +- 13 files changed, 87 insertions(+), 59 deletions(-) diff --git a/bazel/grpc_build_system.bzl b/bazel/grpc_build_system.bzl index c944d8f177b..557f1559919 100644 --- a/bazel/grpc_build_system.bzl +++ b/bazel/grpc_build_system.bzl @@ -100,6 +100,11 @@ def grpc_cc_library( "//:grpc_use_absl": ["@com_google_absl//absl/container:inlined_vector"], "//conditions:default": [], }) + if name == "gpr_base": + more_external_deps += select({ + "//:grpc_use_absl": ["@com_google_absl//absl/strings:strings"], + "//conditions:default": [], + }) native.cc_library( name = name, diff --git a/src/core/ext/filters/client_channel/client_channel.cc b/src/core/ext/filters/client_channel/client_channel.cc index ed61537cb03..41f1ab7a8e5 100644 --- a/src/core/ext/filters/client_channel/client_channel.cc +++ b/src/core/ext/filters/client_channel/client_channel.cc @@ -403,8 +403,9 @@ class CallData { intptr_t handle) const override { grpc_linked_mdelem* linked_mdelem = reinterpret_cast(handle); - return std::make_pair(StringView(GRPC_MDKEY(linked_mdelem->md)), - StringView(GRPC_MDVALUE(linked_mdelem->md))); + return std::make_pair( + StringViewFromSlice(GRPC_MDKEY(linked_mdelem->md)), + StringViewFromSlice(GRPC_MDVALUE(linked_mdelem->md))); } CallData* calld_; diff --git a/src/core/ext/filters/client_channel/xds/xds_client.cc b/src/core/ext/filters/client_channel/xds/xds_client.cc index 9b2b13d28db..3b1e535426b 100644 --- a/src/core/ext/filters/client_channel/xds/xds_client.cc +++ b/src/core/ext/filters/client_channel/xds/xds_client.cc @@ -1257,7 +1257,7 @@ XdsClient::XdsClient(Combiner* combiner, grpc_pollset_set* interested_parties, combiner_(GRPC_COMBINER_REF(combiner, "xds_client")), interested_parties_(interested_parties), bootstrap_(XdsBootstrap::ReadFromFile(error)), - server_name_(server_name.dup()), + server_name_(StringViewToCString(server_name)), service_config_watcher_(std::move(watcher)) { if (*error != GRPC_ERROR_NONE) { if (GRPC_TRACE_FLAG_ENABLED(grpc_xds_client_trace)) { @@ -1296,7 +1296,7 @@ void XdsClient::WatchClusterData(StringView cluster, // TODO(juanlishen): Start CDS call if not already started and return // real data via watcher. CdsUpdate update; - update.eds_service_name = cluster.dup(); + update.eds_service_name = StringViewToCString(cluster); update.lrs_load_reporting_server_name.reset(gpr_strdup("")); w->OnClusterChanged(std::move(update)); } diff --git a/src/core/lib/gprpp/host_port.cc b/src/core/lib/gprpp/host_port.cc index e7f0e4461e9..d69dd9c447f 100644 --- a/src/core/lib/gprpp/host_port.cc +++ b/src/core/lib/gprpp/host_port.cc @@ -57,7 +57,7 @@ bool DoSplitHostPort(StringView name, StringView* host, StringView* port, } if (rbracket == name.size() - 1) { /* ] */ - port->clear(); + *port = StringView(); } else if (name[rbracket + 1] == ':') { /* ]: */ *port = name.substr(rbracket + 2, name.size() - rbracket - 2); @@ -70,7 +70,7 @@ bool DoSplitHostPort(StringView name, StringView* host, StringView* port, if (host->find(':') == grpc_core::StringView::npos) { /* Require all bracketed hosts to contain a colon, because a hostname or IPv4 address should never use brackets. */ - host->clear(); + *host = StringView(); return false; } } else { @@ -84,7 +84,7 @@ bool DoSplitHostPort(StringView name, StringView* host, StringView* port, } else { /* 0 or 2+ colons. Bare hostname or IPv6 litearal. */ *host = name; - port->clear(); + *port = StringView(); } } return true; @@ -108,9 +108,9 @@ bool SplitHostPort(StringView name, UniquePtr* host, // We always set the host, but port is set only when DoSplitHostPort find a // port in the string, to remain backward compatible with the old // gpr_split_host_port API. - *host = host_view.dup(); + *host = StringViewToCString(host_view); if (has_port) { - *port = port_view.dup(); + *port = StringViewToCString(port_view); } } return ret; diff --git a/src/core/lib/gprpp/string_view.h b/src/core/lib/gprpp/string_view.h index 05a81066d48..7823b930709 100644 --- a/src/core/lib/gprpp/string_view.h +++ b/src/core/lib/gprpp/string_view.h @@ -33,8 +33,18 @@ #include "src/core/lib/gpr/useful.h" #include "src/core/lib/gprpp/memory.h" +#if GRPC_USE_ABSL +#include "absl/strings/string_view.h" +#endif + namespace grpc_core { +#if GRPC_USE_ABSL + +using StringView = absl::string_view; + +#else + // Provides a light-weight view over a char array or a slice, similar but not // identical to absl::string_view. // @@ -62,10 +72,6 @@ class StringView final { constexpr StringView(const char* ptr, size_t size) : ptr_(ptr), size_(size) {} constexpr StringView(const char* ptr) : StringView(ptr, ptr == nullptr ? 0 : strlen(ptr)) {} - // Not part of absl::string_view API. - StringView(const grpc_slice& slice) - : StringView(reinterpret_cast(GRPC_SLICE_START_PTR(slice)), - GRPC_SLICE_LENGTH(slice)) {} constexpr StringView() : StringView(nullptr, 0) {} constexpr const char* data() const { return ptr_; } @@ -105,27 +111,6 @@ class StringView final { size_ = 0; } - // Creates a dup of the string viewed by this class. - // Return value is null-terminated and never nullptr. - // - // Not part of absl::string_view API. - grpc_core::UniquePtr dup() const { - char* str = static_cast(gpr_malloc(size_ + 1)); - if (size_ > 0) memcpy(str, ptr_, size_); - str[size_] = '\0'; - return grpc_core::UniquePtr(str); - } - - // Not part of absl::string_view API. - int cmp(StringView other) const { - const size_t len = GPR_MIN(size(), other.size()); - const int ret = strncmp(data(), other.data(), len); - if (ret != 0) return ret; - if (size() == other.size()) return 0; - if (size() < other.size()) return -1; - return 1; - } - private: const char* ptr_; size_t size_; @@ -138,6 +123,33 @@ inline bool operator==(StringView lhs, StringView rhs) { inline bool operator!=(StringView lhs, StringView rhs) { return !(lhs == rhs); } +#endif // GRPC_USE_ABSL + +// Converts grpc_slice to StringView. +inline StringView StringViewFromSlice(const grpc_slice& slice) { + return StringView(reinterpret_cast(GRPC_SLICE_START_PTR(slice)), + GRPC_SLICE_LENGTH(slice)); +} + +// Creates a dup of the string viewed by this class. +// Return value is null-terminated and never nullptr. +inline grpc_core::UniquePtr StringViewToCString(const StringView sv) { + char* str = static_cast(gpr_malloc(sv.size() + 1)); + if (sv.size() > 0) memcpy(str, sv.data(), sv.size()); + str[sv.size()] = '\0'; + return grpc_core::UniquePtr(str); +} + +// Compares lhs and rhs. +inline int StringViewCmp(const StringView lhs, const StringView rhs) { + const size_t len = GPR_MIN(lhs.size(), rhs.size()); + const int ret = strncmp(lhs.data(), rhs.data(), len); + if (ret != 0) return ret; + if (lhs.size() == rhs.size()) return 0; + if (lhs.size() < rhs.size()) return -1; + return 1; +} + } // namespace grpc_core #endif /* GRPC_CORE_LIB_GPRPP_STRING_VIEW_H */ diff --git a/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc b/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc index b28cee3c13b..13b0dd59a00 100644 --- a/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc +++ b/src/core/lib/security/security_connector/ssl/ssl_security_connector.cc @@ -79,7 +79,7 @@ class grpc_ssl_channel_security_connector final grpc_core::StringView host; grpc_core::StringView port; grpc_core::SplitHostPort(target_name, &host, &port); - target_name_ = host.dup(); + target_name_ = grpc_core::StringViewToCString(host); } ~grpc_ssl_channel_security_connector() override { diff --git a/src/core/lib/security/security_connector/ssl_utils.cc b/src/core/lib/security/security_connector/ssl_utils.cc index 66fe80c3c54..570b650eeb6 100644 --- a/src/core/lib/security/security_connector/ssl_utils.cc +++ b/src/core/lib/security/security_connector/ssl_utils.cc @@ -189,9 +189,10 @@ int grpc_ssl_cmp_target_name( grpc_core::StringView target_name, grpc_core::StringView other_target_name, grpc_core::StringView overridden_target_name, grpc_core::StringView other_overridden_target_name) { - int c = target_name.cmp(other_target_name); + int c = grpc_core::StringViewCmp(target_name, other_target_name); if (c != 0) return c; - return overridden_target_name.cmp(other_overridden_target_name); + return grpc_core::StringViewCmp(overridden_target_name, + other_overridden_target_name); } grpc_core::RefCountedPtr grpc_ssl_peer_to_auth_context( diff --git a/src/core/lib/security/security_connector/tls/spiffe_security_connector.cc b/src/core/lib/security/security_connector/tls/spiffe_security_connector.cc index 8dc2937c152..12fd15f6a58 100644 --- a/src/core/lib/security/security_connector/tls/spiffe_security_connector.cc +++ b/src/core/lib/security/security_connector/tls/spiffe_security_connector.cc @@ -127,7 +127,7 @@ SpiffeChannelSecurityConnector::SpiffeChannelSecurityConnector( grpc_core::StringView host; grpc_core::StringView port; grpc_core::SplitHostPort(target_name, &host, &port); - target_name_ = host.dup(); + target_name_ = grpc_core::StringViewToCString(host); } SpiffeChannelSecurityConnector::~SpiffeChannelSecurityConnector() { diff --git a/src/core/lib/security/transport/client_auth_filter.cc b/src/core/lib/security/transport/client_auth_filter.cc index 191f38cac75..2ac73375c75 100644 --- a/src/core/lib/security/transport/client_auth_filter.cc +++ b/src/core/lib/security/transport/client_auth_filter.cc @@ -346,7 +346,8 @@ static void client_auth_start_transport_stream_op_batch( GRPC_CALL_STACK_REF(calld->owning_call, "check_call_host"); GRPC_CLOSURE_INIT(&calld->async_result_closure, on_host_checked, batch, grpc_schedule_on_exec_ctx); - grpc_core::StringView call_host(calld->host); + grpc_core::StringView call_host( + grpc_core::StringViewFromSlice(calld->host)); grpc_error* error = GRPC_ERROR_NONE; if (chand->security_connector->check_call_host( call_host, chand->auth_context.get(), diff --git a/src/core/tsi/ssl_transport_security.cc b/src/core/tsi/ssl_transport_security.cc index f062c458ae8..8b6d9f39dd4 100644 --- a/src/core/tsi/ssl_transport_security.cc +++ b/src/core/tsi/ssl_transport_security.cc @@ -1543,7 +1543,8 @@ static int does_entry_match_name(grpc_core::StringView entry, entry.remove_prefix(2); /* Remove *. */ size_t dot = name_subdomain.find('.'); if (dot == grpc_core::StringView::npos || dot == name_subdomain.size() - 1) { - grpc_core::UniquePtr name_subdomain_cstr(name_subdomain.dup()); + grpc_core::UniquePtr name_subdomain_cstr( + grpc_core::StringViewToCString(name_subdomain)); gpr_log(GPR_ERROR, "Invalid toplevel subdomain: %s", name_subdomain_cstr.get()); return 0; diff --git a/test/core/gprpp/string_view_test.cc b/test/core/gprpp/string_view_test.cc index 1c8adb1db14..d0a1d3cb724 100644 --- a/test/core/gprpp/string_view_test.cc +++ b/test/core/gprpp/string_view_test.cc @@ -40,7 +40,8 @@ TEST(StringViewTest, Empty) { EXPECT_TRUE(empty_trimmed.empty()); EXPECT_EQ(empty_trimmed.size(), 0lu); - grpc_core::StringView empty_slice(grpc_empty_slice()); + grpc_core::StringView empty_slice( + grpc_core::StringViewFromSlice(grpc_empty_slice())); EXPECT_TRUE(empty_slice.empty()); EXPECT_EQ(empty_slice.size(), 0lu); } @@ -64,14 +65,16 @@ TEST(StringViewTest, Data) { TEST(StringViewTest, Slice) { constexpr char kStr[] = "foo"; - grpc_core::StringView slice(grpc_slice_from_static_string(kStr)); + grpc_core::StringView slice( + grpc_core::StringViewFromSlice(grpc_slice_from_static_string(kStr))); EXPECT_EQ(slice.size(), strlen(kStr)); } TEST(StringViewTest, Dup) { constexpr char kStr[] = "foo"; - grpc_core::StringView slice(grpc_slice_from_static_string(kStr)); - grpc_core::UniquePtr dup = slice.dup(); + grpc_core::StringView slice( + grpc_core::StringViewFromSlice(grpc_slice_from_static_string(kStr))); + grpc_core::UniquePtr dup = grpc_core::StringViewToCString(slice); EXPECT_EQ(0, strcmp(kStr, dup.get())); EXPECT_EQ(slice.size(), strlen(kStr)); } @@ -82,12 +85,14 @@ TEST(StringViewTest, Eq) { grpc_core::StringView str1(kStr1); EXPECT_EQ(kStr1, str1); EXPECT_EQ(str1, kStr1); - grpc_core::StringView slice1(grpc_slice_from_static_string(kStr1)); + grpc_core::StringView slice1( + grpc_core::StringViewFromSlice(grpc_slice_from_static_string(kStr1))); EXPECT_EQ(slice1, str1); EXPECT_EQ(str1, slice1); EXPECT_NE(slice1, kStr2); EXPECT_NE(kStr2, slice1); - grpc_core::StringView slice2(grpc_slice_from_static_string(kStr2)); + grpc_core::StringView slice2( + grpc_core::StringViewFromSlice(grpc_slice_from_static_string(kStr2))); EXPECT_NE(slice2, str1); EXPECT_NE(str1, slice2); } @@ -99,15 +104,15 @@ TEST(StringViewTest, Cmp) { grpc_core::StringView str1(kStr1); grpc_core::StringView str2(kStr2); grpc_core::StringView str3(kStr3); - EXPECT_EQ(str1.cmp(str1), 0); - EXPECT_LT(str1.cmp(str2), 0); - EXPECT_LT(str1.cmp(str3), 0); - EXPECT_EQ(str2.cmp(str2), 0); - EXPECT_GT(str2.cmp(str1), 0); - EXPECT_GT(str2.cmp(str3), 0); - EXPECT_EQ(str3.cmp(str3), 0); - EXPECT_GT(str3.cmp(str1), 0); - EXPECT_LT(str3.cmp(str2), 0); + EXPECT_EQ(grpc_core::StringViewCmp(str1, str1), 0); + EXPECT_LT(grpc_core::StringViewCmp(str1, str2), 0); + EXPECT_LT(grpc_core::StringViewCmp(str1, str3), 0); + EXPECT_EQ(grpc_core::StringViewCmp(str2, str2), 0); + EXPECT_GT(grpc_core::StringViewCmp(str2, str1), 0); + EXPECT_GT(grpc_core::StringViewCmp(str2, str3), 0); + EXPECT_EQ(grpc_core::StringViewCmp(str3, str3), 0); + EXPECT_GT(grpc_core::StringViewCmp(str3, str1), 0); + EXPECT_LT(grpc_core::StringViewCmp(str3, str2), 0); } TEST(StringViewTest, RemovePrefix) { diff --git a/test/core/security/credentials_test.cc b/test/core/security/credentials_test.cc index 955afb3b071..884f93fb255 100644 --- a/test/core/security/credentials_test.cc +++ b/test/core/security/credentials_test.cc @@ -748,8 +748,8 @@ static void test_valid_sts_creds_options(void) { grpc_core::StringView host; grpc_core::StringView port; GPR_ASSERT(grpc_core::SplitHostPort(sts_url->authority, &host, &port)); - GPR_ASSERT(host.cmp("foo.com") == 0); - GPR_ASSERT(port.cmp("5555") == 0); + GPR_ASSERT(grpc_core::StringViewCmp(host, "foo.com") == 0); + GPR_ASSERT(grpc_core::StringViewCmp(port, "5555") == 0); grpc_uri_destroy(sts_url); } diff --git a/test/cpp/end2end/client_lb_end2end_test.cc b/test/cpp/end2end/client_lb_end2end_test.cc index 8c3ef368d58..2b62a866740 100644 --- a/test/cpp/end2end/client_lb_end2end_test.cc +++ b/test/cpp/end2end/client_lb_end2end_test.cc @@ -1669,11 +1669,13 @@ class ClientLbInterceptTrailingMetadataTest : public ClientLbEnd2endTest { backend_metric_data->mem_utilization); self->load_report_->set_rps(backend_metric_data->requests_per_second); for (const auto& p : backend_metric_data->request_cost) { - grpc_core::UniquePtr name = p.first.dup(); + grpc_core::UniquePtr name = + grpc_core::StringViewToCString(p.first); (*self->load_report_->mutable_request_cost())[name.get()] = p.second; } for (const auto& p : backend_metric_data->utilization) { - grpc_core::UniquePtr name = p.first.dup(); + grpc_core::UniquePtr name = + grpc_core::StringViewToCString(p.first); (*self->load_report_->mutable_utilization())[name.get()] = p.second; } }