From a2d85e4565029135a06f432d8ccdcc97ce3234df Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Mon, 20 Mar 2023 17:37:22 +0100 Subject: [PATCH] Use absl::string_view type as parameter for RTCError message Bug: webrtc:13579 Change-Id: Ia9f90e6c3b008fc614d378cae4c407becfc597c9 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/298447 Reviewed-by: Tomas Gunnarsson Commit-Queue: Danil Chapovalov Cr-Commit-Position: refs/heads/main@{#39610} --- api/BUILD.gn | 5 ++- api/rtc_error.cc | 20 +++++---- api/rtc_error.h | 11 ++--- api/rtc_error_unittest.cc | 89 ++++++++++++++++++--------------------- pc/sdp_offer_answer.cc | 2 +- pc/sdp_serializer.cc | 3 +- 6 files changed, 65 insertions(+), 65 deletions(-) diff --git a/api/BUILD.gn b/api/BUILD.gn index 6ba5e8afb0..2ad89c9202 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -379,7 +379,10 @@ rtc_library("rtc_error") { "../rtc_base:macromagic", "../rtc_base/system:rtc_export", ] - absl_deps = [ "//third_party/abseil-cpp/absl/types:optional" ] + absl_deps = [ + "//third_party/abseil-cpp/absl/strings", + "//third_party/abseil-cpp/absl/types:optional", + ] } rtc_source_set("packet_socket_factory") { diff --git a/api/rtc_error.cc b/api/rtc_error.cc index 4d3033baf5..0aa4304386 100644 --- a/api/rtc_error.cc +++ b/api/rtc_error.cc @@ -10,11 +10,13 @@ #include "api/rtc_error.h" -#include "rtc_base/arraysize.h" +#include + +#include "absl/strings/string_view.h" namespace { -const char* kRTCErrorTypeNames[] = { +absl::string_view kRTCErrorTypeNames[] = { "NONE", "UNSUPPORTED_OPERATION", "UNSUPPORTED_PARAMETER", @@ -30,11 +32,11 @@ const char* kRTCErrorTypeNames[] = { }; static_assert( static_cast(webrtc::RTCErrorType::OPERATION_ERROR_WITH_DATA) == - (arraysize(kRTCErrorTypeNames) - 1), + (std::size(kRTCErrorTypeNames) - 1), "kRTCErrorTypeNames must have as many strings as RTCErrorType " "has values."); -const char* kRTCErrorDetailTypeNames[] = { +absl::string_view kRTCErrorDetailTypeNames[] = { "NONE", "DATA_CHANNEL_FAILURE", "DTLS_FAILURE", @@ -46,7 +48,7 @@ const char* kRTCErrorDetailTypeNames[] = { }; static_assert( static_cast(webrtc::RTCErrorDetailType::HARDWARE_ENCODER_ERROR) == - (arraysize(kRTCErrorDetailTypeNames) - 1), + (std::size(kRTCErrorDetailTypeNames) - 1), "kRTCErrorDetailTypeNames must have as many strings as " "RTCErrorDetailType has values."); @@ -63,16 +65,16 @@ const char* RTCError::message() const { return message_.c_str(); } -void RTCError::set_message(std::string message) { - message_ = std::move(message); +void RTCError::set_message(absl::string_view message) { + message_ = std::string(message); } -const char* ToString(RTCErrorType error) { +absl::string_view ToString(RTCErrorType error) { int index = static_cast(error); return kRTCErrorTypeNames[index]; } -const char* ToString(RTCErrorDetailType error) { +absl::string_view ToString(RTCErrorDetailType error) { int index = static_cast(error); return kRTCErrorDetailTypeNames[index]; } diff --git a/api/rtc_error.h b/api/rtc_error.h index 42ceed18d9..00b0855bec 100644 --- a/api/rtc_error.h +++ b/api/rtc_error.h @@ -17,6 +17,7 @@ #include #include // For std::move. +#include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" @@ -108,8 +109,8 @@ class RTC_EXPORT RTCError { RTCError() {} explicit RTCError(RTCErrorType type) : type_(type) {} - RTCError(RTCErrorType type, std::string message) - : type_(type), message_(std::move(message)) {} + RTCError(RTCErrorType type, absl::string_view message) + : type_(type), message_(message) {} // In many use cases, it is better to use move than copy, // but copy and assignment are provided for those cases that need it. @@ -133,7 +134,7 @@ class RTC_EXPORT RTCError { // stable. const char* message() const; - void set_message(std::string message); + void set_message(absl::string_view message); RTCErrorDetailType error_detail() const { return error_detail_; } void set_error_detail(RTCErrorDetailType detail) { error_detail_ = detail; } @@ -158,8 +159,8 @@ class RTC_EXPORT RTCError { // // Only intended to be used for logging/diagnostics. The returned char* points // to literal string that lives for the whole duration of the program. -RTC_EXPORT const char* ToString(RTCErrorType error); -RTC_EXPORT const char* ToString(RTCErrorDetailType error); +RTC_EXPORT absl::string_view ToString(RTCErrorType error); +RTC_EXPORT absl::string_view ToString(RTCErrorDetailType error); #ifdef WEBRTC_UNIT_TEST inline std::ostream& operator<<( // no-presubmit-check TODO(webrtc:8982) diff --git a/api/rtc_error_unittest.cc b/api/rtc_error_unittest.cc index ba307d8f71..29dd002b14 100644 --- a/api/rtc_error_unittest.cc +++ b/api/rtc_error_unittest.cc @@ -14,9 +14,10 @@ #include "test/gtest.h" +namespace webrtc { namespace { -const int kDefaultMoveOnlyIntValue = 0xbadf00d; +constexpr int kDefaultMoveOnlyIntValue = 0xbadf00d; // Class that has no copy constructor, ensuring that RTCErrorOr can struct MoveOnlyInt { @@ -55,46 +56,47 @@ struct MoveOnlyInt2 { int value = kDefaultMoveOnlyIntValue; }; -} // namespace - -namespace webrtc { - // Test that the default constructor creates a "no error" error. TEST(RTCErrorTest, DefaultConstructor) { RTCError e; - EXPECT_EQ(RTCErrorType::NONE, e.type()); - EXPECT_EQ(std::string(), e.message()); + EXPECT_EQ(e.type(), RTCErrorType::NONE); + EXPECT_STREQ(e.message(), ""); EXPECT_TRUE(e.ok()); } TEST(RTCErrorTest, NormalConstructors) { RTCError a(RTCErrorType::INVALID_PARAMETER); - EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, a.type()); - EXPECT_EQ(std::string(), a.message()); + EXPECT_EQ(a.type(), RTCErrorType::INVALID_PARAMETER); + EXPECT_STREQ(a.message(), ""); // Constructor that takes const char* message. RTCError b(RTCErrorType::UNSUPPORTED_PARAMETER, "foobar"); - EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, b.type()); - EXPECT_EQ(std::string("foobar"), b.message()); + EXPECT_EQ(b.type(), RTCErrorType::UNSUPPORTED_PARAMETER); + EXPECT_STREQ(b.message(), "foobar"); + + // Constructor that takes absl::string_view message. + RTCError c(RTCErrorType::SYNTAX_ERROR, absl::string_view("baz")); + EXPECT_EQ(c.type(), RTCErrorType::SYNTAX_ERROR); + EXPECT_STREQ(c.message(), "baz"); // Constructor that takes std::string message. - RTCError c(RTCErrorType::INVALID_RANGE, std::string("new")); - EXPECT_EQ(RTCErrorType::INVALID_RANGE, c.type()); - EXPECT_EQ(std::string("new"), c.message()); + RTCError d(RTCErrorType::INVALID_RANGE, std::string("new")); + EXPECT_EQ(d.type(), RTCErrorType::INVALID_RANGE); + EXPECT_STREQ(d.message(), "new"); } TEST(RTCErrorTest, MoveConstructor) { // Static string. RTCError a(RTCErrorType::INVALID_PARAMETER, "foo"); RTCError b(std::move(a)); - EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, b.type()); - EXPECT_EQ(std::string("foo"), b.message()); + EXPECT_EQ(b.type(), RTCErrorType::INVALID_PARAMETER); + EXPECT_STREQ(b.message(), "foo"); // Non-static string. RTCError c(RTCErrorType::UNSUPPORTED_PARAMETER, std::string("bar")); RTCError d(std::move(c)); - EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, d.type()); - EXPECT_EQ(std::string("bar"), d.message()); + EXPECT_EQ(d.type(), RTCErrorType::UNSUPPORTED_PARAMETER); + EXPECT_STREQ(d.message(), "bar"); } TEST(RTCErrorTest, MoveAssignment) { @@ -102,24 +104,21 @@ TEST(RTCErrorTest, MoveAssignment) { RTCError e(RTCErrorType::INVALID_PARAMETER, "foo"); e = RTCError(RTCErrorType::UNSUPPORTED_PARAMETER, "bar"); - EXPECT_EQ(RTCErrorType::UNSUPPORTED_PARAMETER, e.type()); - EXPECT_EQ(std::string("bar"), e.message()); + EXPECT_EQ(e.type(), RTCErrorType::UNSUPPORTED_PARAMETER); + EXPECT_STREQ(e.message(), "bar"); - e = RTCError(RTCErrorType::SYNTAX_ERROR, std::string("baz")); - EXPECT_EQ(std::string("baz"), e.message()); + e = RTCError(RTCErrorType::SYNTAX_ERROR, absl::string_view("baz")); + EXPECT_STREQ(e.message(), "baz"); e = RTCError(RTCErrorType::SYNTAX_ERROR, std::string("another")); - EXPECT_EQ(std::string("another"), e.message()); - - e = RTCError(RTCErrorType::SYNTAX_ERROR, "last"); - EXPECT_EQ(std::string("last"), e.message()); + EXPECT_STREQ(e.message(), "another"); } // Test that the error returned by RTCError::OK() is a "no error" error. TEST(RTCErrorTest, OKConstant) { RTCError ok = RTCError::OK(); - EXPECT_EQ(RTCErrorType::NONE, ok.type()); - EXPECT_EQ(std::string(), ok.message()); + EXPECT_EQ(ok.type(), RTCErrorType::NONE); + EXPECT_STREQ(ok.message(), ""); EXPECT_TRUE(ok.ok()); } @@ -135,33 +134,26 @@ TEST(RTCErrorTest, OkMethod) { // std::strings. TEST(RTCErrorTest, SetMessage) { RTCError e; - // Try all combinations of "is static string"/"is non-static string" calls. e.set_message("foo"); - EXPECT_EQ(std::string("foo"), e.message()); + EXPECT_STREQ(e.message(), "foo"); - e.set_message("bar"); - EXPECT_EQ(std::string("bar"), e.message()); + e.set_message(absl::string_view("bar")); + EXPECT_STREQ(e.message(), "bar"); e.set_message(std::string("string")); - EXPECT_EQ(std::string("string"), e.message()); - - e.set_message(std::string("more")); - EXPECT_EQ(std::string("more"), e.message()); - - e.set_message("love to test"); - EXPECT_EQ(std::string("love to test"), e.message()); + EXPECT_STREQ(e.message(), "string"); } // Test that the default constructor creates an "INTERNAL_ERROR". TEST(RTCErrorOrTest, DefaultConstructor) { RTCErrorOr e; - EXPECT_EQ(RTCErrorType::INTERNAL_ERROR, e.error().type()); + EXPECT_EQ(e.error().type(), RTCErrorType::INTERNAL_ERROR); } // Test that an RTCErrorOr can be implicitly constructed from a value. TEST(RTCErrorOrTest, ImplicitValueConstructor) { RTCErrorOr e = [] { return MoveOnlyInt(100); }(); - EXPECT_EQ(100, e.value().value); + EXPECT_EQ(e.value().value, 100); } // Test that an RTCErrorOr can be implicitly constructed from an RTCError. @@ -169,20 +161,20 @@ TEST(RTCErrorOrTest, ImplicitErrorConstructor) { RTCErrorOr e = [] { return RTCError(RTCErrorType::SYNTAX_ERROR); }(); - EXPECT_EQ(RTCErrorType::SYNTAX_ERROR, e.error().type()); + EXPECT_EQ(e.error().type(), RTCErrorType::SYNTAX_ERROR); } TEST(RTCErrorOrTest, MoveConstructor) { RTCErrorOr a(MoveOnlyInt(5)); RTCErrorOr b(std::move(a)); - EXPECT_EQ(5, b.value().value); + EXPECT_EQ(b.value().value, 5); } TEST(RTCErrorOrTest, MoveAssignment) { RTCErrorOr a(MoveOnlyInt(5)); RTCErrorOr b(MoveOnlyInt(10)); a = std::move(b); - EXPECT_EQ(10, a.value().value); + EXPECT_EQ(a.value().value, 10); } TEST(RTCErrorOrTest, ConversionConstructor) { @@ -194,7 +186,7 @@ TEST(RTCErrorOrTest, ConversionAssignment) { RTCErrorOr a(MoveOnlyInt(5)); RTCErrorOr b(MoveOnlyInt2(10)); b = std::move(a); - EXPECT_EQ(5, b.value().value); + EXPECT_EQ(b.value().value, 5); } TEST(RTCErrorOrTest, OkMethod) { @@ -207,14 +199,14 @@ TEST(RTCErrorOrTest, OkMethod) { TEST(RTCErrorOrTest, MoveError) { RTCErrorOr e({RTCErrorType::SYNTAX_ERROR, "message"}); RTCError err = e.MoveError(); - EXPECT_EQ(RTCErrorType::SYNTAX_ERROR, err.type()); - EXPECT_EQ(std::string("message"), err.message()); + EXPECT_EQ(err.type(), RTCErrorType::SYNTAX_ERROR); + EXPECT_STREQ(err.message(), "message"); } TEST(RTCErrorOrTest, MoveValue) { RTCErrorOr e(MoveOnlyInt(88)); MoveOnlyInt value = e.MoveValue(); - EXPECT_EQ(88, value.value); + EXPECT_EQ(value.value, 88); } // Death tests. @@ -239,4 +231,5 @@ TEST(RTCErrorOrDeathTest, MoveErrorValue) { #endif // RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) +} // namespace } // namespace webrtc diff --git a/pc/sdp_offer_answer.cc b/pc/sdp_offer_answer.cc index 0cfa147e3d..a80f297c85 100644 --- a/pc/sdp_offer_answer.cc +++ b/pc/sdp_offer_answer.cc @@ -856,7 +856,7 @@ class SdpOfferAnswerHandler::RemoteDescriptionOperation { std::string error_message = GetSetDescriptionErrorMessage(cricket::CS_REMOTE, type_, error_); RTC_LOG(LS_ERROR) << error_message; - error_.set_message(std::move(error_message)); + error_.set_message(error_message); } observer_->OnSetRemoteDescriptionComplete(error_); diff --git a/pc/sdp_serializer.cc b/pc/sdp_serializer.cc index 6d405d07a9..31c624b12c 100644 --- a/pc/sdp_serializer.cc +++ b/pc/sdp_serializer.cc @@ -17,6 +17,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/strings/string_view.h" #include "absl/types/optional.h" #include "modules/rtp_rtcp/include/rtp_rtcp_defines.h" #include "rtc_base/checks.h" @@ -52,7 +53,7 @@ const char kSendDirection[] = "send"; const char kReceiveDirection[] = "recv"; const char kPayloadType[] = "pt"; -RTCError ParseError(const std::string& message) { +RTCError ParseError(absl::string_view message) { return RTCError(RTCErrorType::SYNTAX_ERROR, message); }