From ebc4d3edd53febda636eaa6f970d60bcefe2025e Mon Sep 17 00:00:00 2001 From: Sergey Silkin Date: Wed, 15 Nov 2023 11:04:48 +0100 Subject: [PATCH] Move StrJoin to rtc_base/strings A similar function was defined in rtc_base/openssl_adapter. Moving it from net/dcsctp/common/ to rtc_base/strings/. I'm planning to use StrJoin in a video codec test (a follow-up change). Bug: webrtc:14852 Change-Id: Ie657c03e7f9fb52c189c127af6f66ec505b512ae Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/327322 Reviewed-by: Mirko Bonadei Commit-Queue: Sergey Silkin Reviewed-by: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#41166} --- g3doc/style-guide.md | 3 ++- net/dcsctp/common/BUILD.gn | 8 -------- net/dcsctp/packet/BUILD.gn | 3 --- net/dcsctp/packet/chunk/sack_chunk.cc | 2 +- .../missing_mandatory_parameter_cause.cc | 4 ++-- .../supported_extensions_parameter.cc | 4 ++-- net/dcsctp/rx/BUILD.gn | 2 +- net/dcsctp/rx/reassembly_queue.cc | 2 +- net/dcsctp/socket/BUILD.gn | 6 +++--- net/dcsctp/socket/stream_reset_handler.cc | 2 +- net/dcsctp/tx/BUILD.gn | 7 +++---- net/dcsctp/tx/retransmission_queue.cc | 2 +- net/dcsctp/tx/rr_send_queue.cc | 2 +- net/dcsctp/tx/stream_scheduler.cc | 2 +- rtc_base/BUILD.gn | 2 ++ rtc_base/openssl_adapter.cc | 20 ++----------------- rtc_base/openssl_adapter.h | 8 -------- rtc_base/openssl_adapter_unittest.cc | 15 -------------- .../common => rtc_base/strings}/str_join.h | 10 +++++----- .../strings/str_join_unittest.cc | 8 ++++---- 20 files changed, 32 insertions(+), 80 deletions(-) rename {net/dcsctp/common => rtc_base/strings}/str_join.h (88%) rename net/dcsctp/common/str_join_test.cc => rtc_base/strings/str_join_unittest.cc (92%) diff --git a/g3doc/style-guide.md b/g3doc/style-guide.md index b32163f906..8e6570a786 100644 --- a/g3doc/style-guide.md +++ b/g3doc/style-guide.md @@ -132,7 +132,8 @@ docs. WebRTC uses std::string, with content assumed to be UTF-8. Note that this has to be verified whenever accepting external input. -For concatenation of strings, use rtc::SimpleStringBuilder. +For concatenation of strings, use webrtc::StrJoin or rtc::SimpleStringBuilder +directly. The following string building tools are NOT recommended: * The + operator. See https://abseil.io/tips/3 for why not. diff --git a/net/dcsctp/common/BUILD.gn b/net/dcsctp/common/BUILD.gn index 78fa0d307e..d496c64a56 100644 --- a/net/dcsctp/common/BUILD.gn +++ b/net/dcsctp/common/BUILD.gn @@ -29,12 +29,6 @@ rtc_source_set("sequence_numbers") { sources = [ "sequence_numbers.h" ] } -rtc_source_set("str_join") { - deps = [ "../../../rtc_base:stringutils" ] - sources = [ "str_join.h" ] - absl_deps = [ "//third_party/abseil-cpp/absl/strings" ] -} - if (rtc_include_tests) { rtc_library("dcsctp_common_unittests") { testonly = true @@ -43,7 +37,6 @@ if (rtc_include_tests) { deps = [ ":math", ":sequence_numbers", - ":str_join", "../../../api:array_view", "../../../rtc_base:checks", "../../../rtc_base:gunit_helpers", @@ -52,7 +45,6 @@ if (rtc_include_tests) { sources = [ "math_test.cc", "sequence_numbers_test.cc", - "str_join_test.cc", ] } } diff --git a/net/dcsctp/packet/BUILD.gn b/net/dcsctp/packet/BUILD.gn index 7abccc004b..a0c9d8d4df 100644 --- a/net/dcsctp/packet/BUILD.gn +++ b/net/dcsctp/packet/BUILD.gn @@ -72,7 +72,6 @@ rtc_library("parameter") { "../../../rtc_base:stringutils", "../common:internal_types", "../common:math", - "../common:str_join", "../public:types", ] sources = [ @@ -120,7 +119,6 @@ rtc_library("error_cause") { "../../../rtc_base:stringutils", "../common:internal_types", "../common:math", - "../common:str_join", "../packet:bounded_io", "../public:types", ] @@ -172,7 +170,6 @@ rtc_library("chunk") { "../../../rtc_base:logging", "../../../rtc_base:stringutils", "../common:math", - "../common:str_join", "../packet:bounded_io", ] sources = [ diff --git a/net/dcsctp/packet/chunk/sack_chunk.cc b/net/dcsctp/packet/chunk/sack_chunk.cc index d80e430082..179f7ea379 100644 --- a/net/dcsctp/packet/chunk/sack_chunk.cc +++ b/net/dcsctp/packet/chunk/sack_chunk.cc @@ -18,11 +18,11 @@ #include "absl/types/optional.h" #include "api/array_view.h" -#include "net/dcsctp/common/str_join.h" #include "net/dcsctp/packet/bounded_byte_reader.h" #include "net/dcsctp/packet/bounded_byte_writer.h" #include "net/dcsctp/packet/tlv_trait.h" #include "rtc_base/logging.h" +#include "rtc_base/strings/str_join.h" #include "rtc_base/strings/string_builder.h" namespace dcsctp { diff --git a/net/dcsctp/packet/error_cause/missing_mandatory_parameter_cause.cc b/net/dcsctp/packet/error_cause/missing_mandatory_parameter_cause.cc index b89f86e43e..679439d4c2 100644 --- a/net/dcsctp/packet/error_cause/missing_mandatory_parameter_cause.cc +++ b/net/dcsctp/packet/error_cause/missing_mandatory_parameter_cause.cc @@ -18,11 +18,11 @@ #include "absl/types/optional.h" #include "api/array_view.h" -#include "net/dcsctp/common/str_join.h" #include "net/dcsctp/packet/bounded_byte_reader.h" #include "net/dcsctp/packet/bounded_byte_writer.h" #include "net/dcsctp/packet/tlv_trait.h" #include "rtc_base/logging.h" +#include "rtc_base/strings/str_join.h" #include "rtc_base/strings/string_builder.h" namespace dcsctp { @@ -83,7 +83,7 @@ void MissingMandatoryParameterCause::SerializeTo( std::string MissingMandatoryParameterCause::ToString() const { rtc::StringBuilder sb; sb << "Missing Mandatory Parameter, missing_parameter_types=" - << StrJoin(missing_parameter_types_, ","); + << webrtc::StrJoin(missing_parameter_types_, ","); return sb.Release(); } diff --git a/net/dcsctp/packet/parameter/supported_extensions_parameter.cc b/net/dcsctp/packet/parameter/supported_extensions_parameter.cc index 6a8fb214de..87a5bd9b52 100644 --- a/net/dcsctp/packet/parameter/supported_extensions_parameter.cc +++ b/net/dcsctp/packet/parameter/supported_extensions_parameter.cc @@ -16,10 +16,10 @@ #include "absl/types/optional.h" #include "api/array_view.h" -#include "net/dcsctp/common/str_join.h" #include "net/dcsctp/packet/bounded_byte_reader.h" #include "net/dcsctp/packet/bounded_byte_writer.h" #include "net/dcsctp/packet/tlv_trait.h" +#include "rtc_base/strings/str_join.h" #include "rtc_base/strings/string_builder.h" namespace dcsctp { @@ -59,7 +59,7 @@ void SupportedExtensionsParameter::SerializeTo( std::string SupportedExtensionsParameter::ToString() const { rtc::StringBuilder sb; - sb << "Supported Extensions (" << StrJoin(chunk_types_, ", ") << ")"; + sb << "Supported Extensions (" << webrtc::StrJoin(chunk_types_, ", ") << ")"; return sb.Release(); } } // namespace dcsctp diff --git a/net/dcsctp/rx/BUILD.gn b/net/dcsctp/rx/BUILD.gn index f5f5b7ed81..15b9f60f3d 100644 --- a/net/dcsctp/rx/BUILD.gn +++ b/net/dcsctp/rx/BUILD.gn @@ -95,10 +95,10 @@ rtc_library("reassembly_queue") { "../../../api:array_view", "../../../rtc_base:checks", "../../../rtc_base:logging", + "../../../rtc_base:stringutils", "../../../rtc_base/containers:flat_set", "../common:internal_types", "../common:sequence_numbers", - "../common:str_join", "../packet:chunk", "../packet:data", "../packet:parameter", diff --git a/net/dcsctp/rx/reassembly_queue.cc b/net/dcsctp/rx/reassembly_queue.cc index 573443635c..ae672731c0 100644 --- a/net/dcsctp/rx/reassembly_queue.cc +++ b/net/dcsctp/rx/reassembly_queue.cc @@ -23,7 +23,6 @@ #include "absl/types/optional.h" #include "api/array_view.h" #include "net/dcsctp/common/sequence_numbers.h" -#include "net/dcsctp/common/str_join.h" #include "net/dcsctp/packet/chunk/forward_tsn_common.h" #include "net/dcsctp/packet/data.h" #include "net/dcsctp/packet/parameter/outgoing_ssn_reset_request_parameter.h" @@ -34,6 +33,7 @@ #include "net/dcsctp/rx/reassembly_streams.h" #include "net/dcsctp/rx/traditional_reassembly_streams.h" #include "rtc_base/logging.h" +#include "rtc_base/strings/str_join.h" namespace dcsctp { namespace { diff --git a/net/dcsctp/socket/BUILD.gn b/net/dcsctp/socket/BUILD.gn index 9820207a37..04f61e5b72 100644 --- a/net/dcsctp/socket/BUILD.gn +++ b/net/dcsctp/socket/BUILD.gn @@ -24,8 +24,8 @@ rtc_library("heartbeat_handler") { deps = [ ":context", "../../../api:array_view", - "../../../rtc_base:checks", "../../../api/units:time_delta", + "../../../rtc_base:checks", "../../../rtc_base:logging", "../packet:bounded_io", "../packet:chunk", @@ -53,9 +53,9 @@ rtc_library("stream_reset_handler") { "../../../api/units:time_delta", "../../../rtc_base:checks", "../../../rtc_base:logging", + "../../../rtc_base:stringutils", "../../../rtc_base/containers:flat_set", "../common:internal_types", - "../common:str_join", "../packet:chunk", "../packet:parameter", "../packet:sctp_packet", @@ -99,8 +99,8 @@ rtc_library("transmission_control_block") { ":packet_sender", ":stream_reset_handler", "../../../api:array_view", - "../../../api/units:time_delta", "../../../api/task_queue:task_queue", + "../../../api/units:time_delta", "../../../rtc_base:checks", "../../../rtc_base:logging", "../../../rtc_base:stringutils", diff --git a/net/dcsctp/socket/stream_reset_handler.cc b/net/dcsctp/socket/stream_reset_handler.cc index f81057de6f..fafb9933e5 100644 --- a/net/dcsctp/socket/stream_reset_handler.cc +++ b/net/dcsctp/socket/stream_reset_handler.cc @@ -18,7 +18,6 @@ #include "api/array_view.h" #include "api/units/time_delta.h" #include "net/dcsctp/common/internal_types.h" -#include "net/dcsctp/common/str_join.h" #include "net/dcsctp/packet/chunk/reconfig_chunk.h" #include "net/dcsctp/packet/parameter/add_incoming_streams_request_parameter.h" #include "net/dcsctp/packet/parameter/add_outgoing_streams_request_parameter.h" @@ -36,6 +35,7 @@ #include "net/dcsctp/timer/timer.h" #include "net/dcsctp/tx/retransmission_queue.h" #include "rtc_base/logging.h" +#include "rtc_base/strings/str_join.h" namespace dcsctp { namespace { diff --git a/net/dcsctp/tx/BUILD.gn b/net/dcsctp/tx/BUILD.gn index 3183be36d8..d1fd8ab3d5 100644 --- a/net/dcsctp/tx/BUILD.gn +++ b/net/dcsctp/tx/BUILD.gn @@ -29,9 +29,9 @@ rtc_library("rr_send_queue") { "../../../api:array_view", "../../../rtc_base:checks", "../../../rtc_base:logging", + "../../../rtc_base:stringutils", "../../../rtc_base/containers:flat_map", "../common:internal_types", - "../common:str_join", "../packet:data", "../public:socket", "../public:types", @@ -54,9 +54,9 @@ rtc_library("stream_scheduler") { "../../../api:array_view", "../../../rtc_base:checks", "../../../rtc_base:logging", + "../../../rtc_base:stringutils", "../../../rtc_base:strong_alias", "../../../rtc_base/containers:flat_set", - "../common:str_join", "../packet:chunk", "../packet:data", "../packet:sctp_packet", @@ -109,11 +109,11 @@ rtc_library("outstanding_data") { "../../../api/units:timestamp", "../../../rtc_base:checks", "../../../rtc_base:logging", + "../../../rtc_base:stringutils", "../../../rtc_base/containers:flat_set", "../common:internal_types", "../common:math", "../common:sequence_numbers", - "../common:str_join", "../packet:chunk", "../packet:data", "../public:socket", @@ -142,7 +142,6 @@ rtc_library("retransmission_queue") { "../../../rtc_base:stringutils", "../common:math", "../common:sequence_numbers", - "../common:str_join", "../packet:chunk", "../packet:data", "../public:socket", diff --git a/net/dcsctp/tx/retransmission_queue.cc b/net/dcsctp/tx/retransmission_queue.cc index da06ddcafd..cd1cc14b4f 100644 --- a/net/dcsctp/tx/retransmission_queue.cc +++ b/net/dcsctp/tx/retransmission_queue.cc @@ -25,7 +25,6 @@ #include "api/array_view.h" #include "net/dcsctp/common/math.h" #include "net/dcsctp/common/sequence_numbers.h" -#include "net/dcsctp/common/str_join.h" #include "net/dcsctp/packet/chunk/data_chunk.h" #include "net/dcsctp/packet/chunk/forward_tsn_chunk.h" #include "net/dcsctp/packet/chunk/forward_tsn_common.h" @@ -40,6 +39,7 @@ #include "net/dcsctp/tx/send_queue.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" +#include "rtc_base/strings/str_join.h" #include "rtc_base/strings/string_builder.h" namespace dcsctp { diff --git a/net/dcsctp/tx/rr_send_queue.cc b/net/dcsctp/tx/rr_send_queue.cc index b889c063d7..7cbead296c 100644 --- a/net/dcsctp/tx/rr_send_queue.cc +++ b/net/dcsctp/tx/rr_send_queue.cc @@ -21,13 +21,13 @@ #include "absl/types/optional.h" #include "api/array_view.h" #include "net/dcsctp/common/internal_types.h" -#include "net/dcsctp/common/str_join.h" #include "net/dcsctp/packet/data.h" #include "net/dcsctp/public/dcsctp_message.h" #include "net/dcsctp/public/dcsctp_socket.h" #include "net/dcsctp/public/types.h" #include "net/dcsctp/tx/send_queue.h" #include "rtc_base/logging.h" +#include "rtc_base/strings/str_join.h" namespace dcsctp { using ::webrtc::TimeDelta; diff --git a/net/dcsctp/tx/stream_scheduler.cc b/net/dcsctp/tx/stream_scheduler.cc index 6c51e1e553..66c4457481 100644 --- a/net/dcsctp/tx/stream_scheduler.cc +++ b/net/dcsctp/tx/stream_scheduler.cc @@ -14,7 +14,6 @@ #include "absl/algorithm/container.h" #include "absl/types/optional.h" #include "api/array_view.h" -#include "net/dcsctp/common/str_join.h" #include "net/dcsctp/packet/data.h" #include "net/dcsctp/public/dcsctp_message.h" #include "net/dcsctp/public/dcsctp_socket.h" @@ -22,6 +21,7 @@ #include "net/dcsctp/tx/send_queue.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" +#include "rtc_base/strings/str_join.h" namespace dcsctp { diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 9cf0522164..899a689e49 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -606,6 +606,7 @@ rtc_library("stringutils") { "string_to_number.h", "string_utils.cc", "string_utils.h", + "strings/str_join.h", "strings/string_builder.cc", "strings/string_builder.h", "strings/string_format.cc", @@ -1909,6 +1910,7 @@ if (rtc_include_tests) { "string_encode_unittest.cc", "string_to_number_unittest.cc", "string_utils_unittest.cc", + "strings/str_join_unittest.cc", "strings/string_builder_unittest.cc", "strings/string_format_unittest.cc", "strong_alias_unittest.cc", diff --git a/rtc_base/openssl_adapter.cc b/rtc_base/openssl_adapter.cc index c68eb22f5c..e48cdf43bd 100644 --- a/rtc_base/openssl_adapter.cc +++ b/rtc_base/openssl_adapter.cc @@ -44,6 +44,7 @@ #include "rtc_base/openssl_identity.h" #endif #include "rtc_base/openssl_utility.h" +#include "rtc_base/strings/str_join.h" #include "rtc_base/strings/string_builder.h" #include "rtc_base/thread.h" @@ -168,23 +169,6 @@ namespace rtc { using ::webrtc::TimeDelta; -namespace webrtc_openssl_adapter_internal { - -// Simple O(n^2) implementation is sufficient for current use case. -std::string StrJoin(const std::vector& list, char delimiter) { - RTC_CHECK(!list.empty()); - StringBuilder sb; - sb << list[0]; - for (size_t i = 1; i < list.size(); i++) { - sb.AppendFormat("%c", delimiter); - sb << list[i]; - } - return sb.Release(); -} -} // namespace webrtc_openssl_adapter_internal - -using webrtc_openssl_adapter_internal::StrJoin; - bool OpenSSLAdapter::InitializeSSL() { if (!SSL_library_init()) return false; @@ -373,7 +357,7 @@ int OpenSSLAdapter::BeginSSL() { } if (!elliptic_curves_.empty()) { - SSL_set1_curves_list(ssl_, StrJoin(elliptic_curves_, ':').c_str()); + SSL_set1_curves_list(ssl_, webrtc::StrJoin(elliptic_curves_, ":").c_str()); } // Now that the initial config is done, transfer ownership of `bio` to the diff --git a/rtc_base/openssl_adapter.h b/rtc_base/openssl_adapter.h index 558a04077a..4c05471b2b 100644 --- a/rtc_base/openssl_adapter.h +++ b/rtc_base/openssl_adapter.h @@ -37,14 +37,6 @@ namespace rtc { -namespace webrtc_openssl_adapter_internal { - -// Local definition, since absl::StrJoin is not allow-listed. Declared in header -// file only for unittests. -std::string StrJoin(const std::vector& list, char delimiter); - -} // namespace webrtc_openssl_adapter_internal - class OpenSSLAdapter final : public SSLAdapter { public: static bool InitializeSSL(); diff --git a/rtc_base/openssl_adapter_unittest.cc b/rtc_base/openssl_adapter_unittest.cc index ce351dc98e..5b59a8019e 100644 --- a/rtc_base/openssl_adapter_unittest.cc +++ b/rtc_base/openssl_adapter_unittest.cc @@ -116,19 +116,4 @@ TEST(OpenSSLAdapterFactoryTest, CreateWorksWithCustomVerifier) { EXPECT_NE(simple_adapter, nullptr); } -TEST(StrJoinTest, SingleElement) { - EXPECT_EQ(webrtc_openssl_adapter_internal::StrJoin({"a"}, ','), "a"); -} - -TEST(StrJoinTest, TwoElements) { - EXPECT_EQ(webrtc_openssl_adapter_internal::StrJoin({"first", "second"}, ':'), - "first:second"); -} - -TEST(StrJoinTest, WithEmptyElement) { - EXPECT_EQ( - webrtc_openssl_adapter_internal::StrJoin({"first", "", "second"}, ':'), - "first::second"); -} - } // namespace rtc diff --git a/net/dcsctp/common/str_join.h b/rtc_base/strings/str_join.h similarity index 88% rename from net/dcsctp/common/str_join.h rename to rtc_base/strings/str_join.h index 04517827b7..762e63ae2a 100644 --- a/net/dcsctp/common/str_join.h +++ b/rtc_base/strings/str_join.h @@ -7,15 +7,15 @@ * in the file PATENTS. All contributing project authors may * be found in the AUTHORS file in the root of the source tree. */ -#ifndef NET_DCSCTP_COMMON_STR_JOIN_H_ -#define NET_DCSCTP_COMMON_STR_JOIN_H_ +#ifndef RTC_BASE_STRINGS_STR_JOIN_H_ +#define RTC_BASE_STRINGS_STR_JOIN_H_ #include #include "absl/strings/string_view.h" #include "rtc_base/strings/string_builder.h" -namespace dcsctp { +namespace webrtc { template std::string StrJoin(const Range& seq, absl::string_view delimiter) { @@ -51,6 +51,6 @@ std::string StrJoin(const Range& seq, return sb.Release(); } -} // namespace dcsctp +} // namespace webrtc -#endif // NET_DCSCTP_COMMON_STR_JOIN_H_ +#endif // RTC_BASE_STRINGS_STR_JOIN_H_ diff --git a/net/dcsctp/common/str_join_test.cc b/rtc_base/strings/str_join_unittest.cc similarity index 92% rename from net/dcsctp/common/str_join_test.cc rename to rtc_base/strings/str_join_unittest.cc index dbfd92c1cf..a4ac02125f 100644 --- a/net/dcsctp/common/str_join_test.cc +++ b/rtc_base/strings/str_join_unittest.cc @@ -7,15 +7,15 @@ * in the file PATENTS. All contributing project authors may * be found in the AUTHORS file in the root of the source tree. */ -#include "net/dcsctp/common/str_join.h" +#include "rtc_base/strings/str_join.h" #include #include #include -#include "test/gmock.h" +#include "test/gtest.h" -namespace dcsctp { +namespace webrtc { namespace { TEST(StrJoinTest, CanJoinStringsFromVector) { @@ -42,4 +42,4 @@ TEST(StrJoinTest, CanFormatElementsWhileJoining) { } } // namespace -} // namespace dcsctp +} // namespace webrtc