From 941a07cca3b1e6ad674fcb435d9f0f83d106f896 Mon Sep 17 00:00:00 2001 From: Jonas Olsson Date: Thu, 13 Sep 2018 10:07:07 +0200 Subject: [PATCH] Remove all remaining non-test uses of std::stringstream. Bug: webrtc:8982 Change-Id: I635a8545c46dc8c89663d64af351e22e65cbcb33 Reviewed-on: https://webrtc-review.googlesource.com/98880 Commit-Queue: Jonas Olsson Reviewed-by: Karl Wiberg Cr-Commit-Position: refs/heads/master@{#24715} --- api/rtcerror.cc | 51 ++----------------- api/rtcerror.h | 28 +++------- api/rtcerror_unittest.cc | 3 +- api/units/time_delta.h | 1 + examples/stunprober/main.cc | 1 + logging/rtc_event_log/rtc_event_log_parser.h | 1 + .../rtc_event_log/rtc_event_log_parser_new.h | 1 + media/engine/webrtcvideoengine.cc | 13 ++--- modules/audio_processing/aec3/aec3_fft.cc | 1 + .../audio_processing/aec3/render_buffer.cc | 1 + modules/audio_processing/test/test_utils.h | 1 + ortc/rtptransportadapter.cc | 1 - ortc/rtptransportcontrolleradapter.cc | 4 +- p2p/base/dtlstransport.h | 11 ++-- p2p/base/p2ptransportchannel.h | 7 +-- p2p/base/port.cc | 49 +++++++++--------- pc/peerconnection.cc | 2 +- rtc_base/BUILD.gn | 3 ++ rtc_base/checks.h | 1 - rtc_base/json.cc | 12 ++--- rtc_base/logging.cc | 2 - rtc_base/logging.h | 2 +- rtc_base/socketaddress.h | 1 - rtc_base/stringencode.h | 1 - test/fuzzers/rtp_packet_fuzzer.cc | 2 + 25 files changed, 74 insertions(+), 126 deletions(-) diff --git a/api/rtcerror.cc b/api/rtcerror.cc index 55ac15e196b..039e7f340af 100644 --- a/api/rtcerror.cc +++ b/api/rtcerror.cc @@ -36,32 +36,8 @@ static_assert(static_cast(webrtc::RTCErrorType::INTERNAL_ERROR) == namespace webrtc { -RTCError::RTCError(RTCError&& other) - : type_(other.type_), have_string_message_(other.have_string_message_) { - if (have_string_message_) { - new (&string_message_) std::string(std::move(other.string_message_)); - } else { - static_message_ = other.static_message_; - } -} - -RTCError& RTCError::operator=(RTCError&& other) { - type_ = other.type_; - if (other.have_string_message_) { - set_message(std::move(other.string_message_)); - } else { - set_message(other.static_message_); - } - return *this; -} - -RTCError::~RTCError() { - // If we hold a message string that was built, rather than a static string, - // we need to delete it. - if (have_string_message_) { - string_message_.~basic_string(); - } -} +RTCError::RTCError(RTCError&& other) = default; +RTCError& RTCError::operator=(RTCError&& other) = default; // static RTCError RTCError::OK() { @@ -69,28 +45,11 @@ RTCError RTCError::OK() { } const char* RTCError::message() const { - if (have_string_message_) { - return string_message_.c_str(); - } else { - return static_message_; - } -} - -void RTCError::set_message(const char* message) { - if (have_string_message_) { - string_message_.~basic_string(); - have_string_message_ = false; - } - static_message_ = message; + return message_.c_str(); } -void RTCError::set_message(std::string&& message) { - if (!have_string_message_) { - new (&string_message_) std::string(std::move(message)); - have_string_message_ = true; - } else { - string_message_ = message; - } +void RTCError::set_message(std::string message) { + message_ = std::move(message); } // TODO(jonasolsson): Change to use absl::string_view when it's available. diff --git a/api/rtcerror.h b/api/rtcerror.h index c87ce916501..4910682dbf3 100644 --- a/api/rtcerror.h +++ b/api/rtcerror.h @@ -86,12 +86,9 @@ class RTCError { // Creates a "no error" error. RTCError() {} explicit RTCError(RTCErrorType type) : type_(type) {} - // For performance, prefer using the constructor that takes a const char* if - // the message is a static string. - RTCError(RTCErrorType type, const char* message) - : type_(type), static_message_(message), have_string_message_(false) {} - RTCError(RTCErrorType type, std::string&& message) - : type_(type), string_message_(message), have_string_message_(true) {} + + RTCError(RTCErrorType type, std::string message) + : type_(type), message_(std::move(message)) {} // Delete the copy constructor and assignment operator; there aren't any use // cases where you should need to copy an RTCError, as opposed to moving it. @@ -103,8 +100,6 @@ class RTCError { RTCError(RTCError&& other); RTCError& operator=(RTCError&& other); - ~RTCError(); - // Identical to default constructed error. // // Preferred over the default constructor for code readability. @@ -118,10 +113,8 @@ class RTCError { // anything but logging/diagnostics, since messages are not guaranteed to be // stable. const char* message() const; - // For performance, prefer using the method that takes a const char* if the - // message is a static string. - void set_message(const char* message); - void set_message(std::string&& message); + + void set_message(std::string message); // Convenience method for situations where you only care whether or not an // error occurred. @@ -129,16 +122,7 @@ class RTCError { private: RTCErrorType type_ = RTCErrorType::NONE; - // For performance, we use static strings wherever possible. But in some - // cases the error string may need to be constructed, in which case an - // std::string is used. - union { - const char* static_message_ = ""; - std::string string_message_; - }; - // Whether or not |static_message_| or |string_message_| is being used in the - // above union. - bool have_string_message_ = false; + std::string message_; }; // Outputs the error as a friendly string. Update this method when adding a new diff --git a/api/rtcerror_unittest.cc b/api/rtcerror_unittest.cc index 90593cf5246..a1ea83f0040 100644 --- a/api/rtcerror_unittest.cc +++ b/api/rtcerror_unittest.cc @@ -222,7 +222,8 @@ TEST(RTCErrorOrTest, MoveValue) { #if RTC_DCHECK_IS_ON && GTEST_HAS_DEATH_TEST && !defined(WEBRTC_ANDROID) TEST(RTCErrorOrDeathTest, ConstructWithOkError) { - EXPECT_DEATH(RTCErrorOr err = RTCError::OK(), ""); + RTCErrorOr err; + EXPECT_DEATH(err = RTCError::OK(), ""); } TEST(RTCErrorOrDeathTest, DereferenceErrorValue) { diff --git a/api/units/time_delta.h b/api/units/time_delta.h index 6e69333fca6..ec364172aa1 100644 --- a/api/units/time_delta.h +++ b/api/units/time_delta.h @@ -17,6 +17,7 @@ #include #include +#include #include #include diff --git a/examples/stunprober/main.cc b/examples/stunprober/main.cc index 42ee0a1a9a2..45a76f52374 100644 --- a/examples/stunprober/main.cc +++ b/examples/stunprober/main.cc @@ -14,6 +14,7 @@ #include #include +#include // no-presubmit-check TODO(webrtc:8982) #include "p2p/base/basicpacketsocketfactory.h" #include "p2p/stunprober/stunprober.h" diff --git a/logging/rtc_event_log/rtc_event_log_parser.h b/logging/rtc_event_log/rtc_event_log_parser.h index 310e5a84a57..c93ec6da7c3 100644 --- a/logging/rtc_event_log/rtc_event_log_parser.h +++ b/logging/rtc_event_log/rtc_event_log_parser.h @@ -11,6 +11,7 @@ #define LOGGING_RTC_EVENT_LOG_RTC_EVENT_LOG_PARSER_H_ #include +#include // no-presubmit-check TODO(webrtc:8982) #include #include // pair #include diff --git a/logging/rtc_event_log/rtc_event_log_parser_new.h b/logging/rtc_event_log/rtc_event_log_parser_new.h index 53d917c5386..bf35ae9440a 100644 --- a/logging/rtc_event_log/rtc_event_log_parser_new.h +++ b/logging/rtc_event_log/rtc_event_log_parser_new.h @@ -13,6 +13,7 @@ #include #include #include +#include // no-presubmit-check TODO(webrtc:8982) #include #include // pair #include diff --git a/media/engine/webrtcvideoengine.cc b/media/engine/webrtcvideoengine.cc index 18919bc30a8..30e367a1cc8 100644 --- a/media/engine/webrtcvideoengine.cc +++ b/media/engine/webrtcvideoengine.cc @@ -34,6 +34,7 @@ #include "modules/video_coding/include/video_error_codes.h" #include "rtc_base/copyonwritebuffer.h" #include "rtc_base/logging.h" +#include "rtc_base/strings/string_builder.h" #include "rtc_base/stringutils.h" #include "rtc_base/timeutils.h" #include "rtc_base/trace_event.h" @@ -168,15 +169,15 @@ std::vector AssignPayloadTypesAndDefaultCodecs( } static std::string CodecVectorToString(const std::vector& codecs) { - std::stringstream out; - out << '{'; + rtc::StringBuilder out; + out << "{"; for (size_t i = 0; i < codecs.size(); ++i) { out << codecs[i].ToString(); if (i != codecs.size() - 1) { out << ", "; } } - out << '}'; + out << "}"; return out.str(); } @@ -940,15 +941,15 @@ bool WebRtcVideoChannel::SetRecvParameters(const VideoRecvParameters& params) { std::string WebRtcVideoChannel::CodecSettingsVectorToString( const std::vector& codecs) { - std::stringstream out; - out << '{'; + rtc::StringBuilder out; + out << "{"; for (size_t i = 0; i < codecs.size(); ++i) { out << codecs[i].codec.ToString(); if (i != codecs.size() - 1) { out << ", "; } } - out << '}'; + out << "}"; return out.str(); } diff --git a/modules/audio_processing/aec3/aec3_fft.cc b/modules/audio_processing/aec3/aec3_fft.cc index b02f3425b71..e29434d3002 100644 --- a/modules/audio_processing/aec3/aec3_fft.cc +++ b/modules/audio_processing/aec3/aec3_fft.cc @@ -11,6 +11,7 @@ #include "modules/audio_processing/aec3/aec3_fft.h" #include +#include #include "rtc_base/checks.h" diff --git a/modules/audio_processing/aec3/render_buffer.cc b/modules/audio_processing/aec3/render_buffer.cc index 235e3e335af..f6ffa046e62 100644 --- a/modules/audio_processing/aec3/render_buffer.cc +++ b/modules/audio_processing/aec3/render_buffer.cc @@ -11,6 +11,7 @@ #include "modules/audio_processing/aec3/render_buffer.h" #include +#include #include "modules/audio_processing/aec3/aec3_common.h" #include "rtc_base/checks.h" diff --git a/modules/audio_processing/test/test_utils.h b/modules/audio_processing/test/test_utils.h index 6e4154ad6d4..10877f1a5f4 100644 --- a/modules/audio_processing/test/test_utils.h +++ b/modules/audio_processing/test/test_utils.h @@ -15,6 +15,7 @@ #include #include #include +#include // no-presubmit-check TODO(webrtc:8982) #include #include diff --git a/ortc/rtptransportadapter.cc b/ortc/rtptransportadapter.cc index 2829427c6c4..84c2ef09e66 100644 --- a/ortc/rtptransportadapter.cc +++ b/ortc/rtptransportadapter.cc @@ -12,7 +12,6 @@ #include // For std::find. #include -#include #include // For std::move. #include "absl/memory/memory.h" diff --git a/ortc/rtptransportcontrolleradapter.cc b/ortc/rtptransportcontrolleradapter.cc index 853adf12745..dbb03ea321f 100644 --- a/ortc/rtptransportcontrolleradapter.cc +++ b/ortc/rtptransportcontrolleradapter.cc @@ -12,7 +12,6 @@ #include // For "remove", "find". #include -#include #include #include // For std::move. @@ -25,6 +24,7 @@ #include "pc/rtpmediautils.h" #include "pc/rtpparametersconversion.h" #include "rtc_base/checks.h" +#include "rtc_base/strings/string_builder.h" namespace webrtc { @@ -39,7 +39,7 @@ static RTCError CheckForIdConflicts( const std::vector& codecs_b, const cricket::RtpHeaderExtensions& extensions_b, const cricket::StreamParamsVec& streams_b) { - std::ostringstream oss; + rtc::StringBuilder oss; // Since it's assumed that C1 and C2 are different types, codecs_a and // codecs_b should never contain the same payload type, and thus we can just // use a set. diff --git a/p2p/base/dtlstransport.h b/p2p/base/dtlstransport.h index 609f3d72150..5c0f795a084 100644 --- a/p2p/base/dtlstransport.h +++ b/p2p/base/dtlstransport.h @@ -22,6 +22,7 @@ #include "rtc_base/constructormagic.h" #include "rtc_base/sslstreamadapter.h" #include "rtc_base/stream.h" +#include "rtc_base/strings/string_builder.h" #include "rtc_base/thread_checker.h" namespace rtc { @@ -180,12 +181,12 @@ class DtlsTransport : public DtlsTransportInternal { int SetOption(rtc::Socket::Option opt, int value) override; std::string ToString() const { - const char RECEIVING_ABBREV[2] = {'_', 'R'}; - const char WRITABLE_ABBREV[2] = {'_', 'W'}; - std::stringstream ss; - ss << "DtlsTransport[" << transport_name_ << "|" << component_ << "|" + const absl::string_view RECEIVING_ABBREV[2] = {"_", "R"}; + const absl::string_view WRITABLE_ABBREV[2] = {"_", "W"}; + rtc::StringBuilder sb; + sb << "DtlsTransport[" << transport_name_ << "|" << component_ << "|" << RECEIVING_ABBREV[receiving()] << WRITABLE_ABBREV[writable()] << "]"; - return ss.str(); + return sb.str(); } private: diff --git a/p2p/base/p2ptransportchannel.h b/p2p/base/p2ptransportchannel.h index 9978cb00708..e4d371f2df1 100644 --- a/p2p/base/p2ptransportchannel.h +++ b/p2p/base/p2ptransportchannel.h @@ -41,6 +41,7 @@ #include "rtc_base/asyncinvoker.h" #include "rtc_base/asyncpacketsocket.h" #include "rtc_base/constructormagic.h" +#include "rtc_base/strings/string_builder.h" #include "rtc_base/third_party/sigslot/sigslot.h" namespace webrtc { @@ -171,9 +172,9 @@ class P2PTransportChannel : public IceTransportInternal { } std::string ToString() const { - const char RECEIVING_ABBREV[2] = {'_', 'R'}; - const char WRITABLE_ABBREV[2] = {'_', 'W'}; - std::stringstream ss; + const std::string RECEIVING_ABBREV[2] = {"_", "R"}; + const std::string WRITABLE_ABBREV[2] = {"_", "W"}; + rtc::StringBuilder ss; ss << "Channel[" << transport_name_ << "|" << component_ << "|" << RECEIVING_ABBREV[receiving_] << WRITABLE_ABBREV[writable_] << "]"; return ss.str(); diff --git a/p2p/base/port.cc b/p2p/base/port.cc index 0ba9bb3e01c..473f623a298 100644 --- a/p2p/base/port.cc +++ b/p2p/base/port.cc @@ -207,9 +207,9 @@ static std::string ComputeFoundation(const std::string& type, const std::string& protocol, const std::string& relay_protocol, const rtc::SocketAddress& base_address) { - std::ostringstream ost; - ost << type << base_address.ipaddr().ToString() << protocol << relay_protocol; - return rtc::ToString(rtc::ComputeCrc32(ost.str())); + rtc::StringBuilder sb; + sb << type << base_address.ipaddr().ToString() << protocol << relay_protocol; + return rtc::ToString(rtc::ComputeCrc32(sb.Release())); } CandidateStats::CandidateStats() = default; @@ -905,10 +905,10 @@ void Port::OnNetworkTypeChanged(const rtc::Network* network) { } std::string Port::ToString() const { - std::stringstream ss; - ss << "Port[" << std::hex << this << std::dec << ":" << content_name_ << ":" - << component_ << ":" << generation_ << ":" << type_ << ":" - << network_->ToString() << "]"; + rtc::StringBuilder ss; + ss << "Port[" << rtc::ToHex(reinterpret_cast(this)) << ":" + << content_name_ << ":" << component_ << ":" << generation_ << ":" << type_ + << ":" << network_->ToString() << "]"; return ss.str(); } @@ -1398,8 +1398,7 @@ void Connection::FailAndPrune() { } void Connection::PrintPingsSinceLastResponse(std::string* s, size_t max) { - std::ostringstream oss; - oss << std::boolalpha; + rtc::StringBuilder oss; if (pings_since_last_response_.size() > max) { for (size_t i = 0; i < max; i++) { const SentPing& ping = pings_since_last_response_[i]; @@ -1561,9 +1560,7 @@ bool Connection::stable(int64_t now) const { } std::string Connection::ToDebugId() const { - std::stringstream ss; - ss << std::hex << this; - return ss.str(); + return rtc::ToHex(reinterpret_cast(this)); } uint32_t Connection::ComputeNetworkCost() const { @@ -1572,33 +1569,33 @@ uint32_t Connection::ComputeNetworkCost() const { } std::string Connection::ToString() const { - const char CONNECT_STATE_ABBREV[2] = { - '-', // not connected (false) - 'C', // connected (true) + const absl::string_view CONNECT_STATE_ABBREV[2] = { + "-", // not connected (false) + "C", // connected (true) }; - const char RECEIVE_STATE_ABBREV[2] = { - '-', // not receiving (false) - 'R', // receiving (true) + const absl::string_view RECEIVE_STATE_ABBREV[2] = { + "-", // not receiving (false) + "R", // receiving (true) }; - const char WRITE_STATE_ABBREV[4] = { - 'W', // STATE_WRITABLE - 'w', // STATE_WRITE_UNRELIABLE - '-', // STATE_WRITE_INIT - 'x', // STATE_WRITE_TIMEOUT + const absl::string_view WRITE_STATE_ABBREV[4] = { + "W", // STATE_WRITABLE + "w", // STATE_WRITE_UNRELIABLE + "-", // STATE_WRITE_INIT + "x", // STATE_WRITE_TIMEOUT }; - const std::string ICESTATE[4] = { + const absl::string_view ICESTATE[4] = { "W", // STATE_WAITING "I", // STATE_INPROGRESS "S", // STATE_SUCCEEDED "F" // STATE_FAILED }; - const std::string SELECTED_STATE_ABBREV[2] = { + const absl::string_view SELECTED_STATE_ABBREV[2] = { "-", // candidate pair not selected (false) "S", // selected (true) }; const Candidate& local = local_candidate(); const Candidate& remote = remote_candidate(); - std::stringstream ss; + rtc::StringBuilder ss; ss << "Conn[" << ToDebugId() << ":" << port_->content_name() << ":" << port_->Network()->ToString() << ":" << local.id() << ":" << local.component() << ":" << local.generation() << ":" << local.type() diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 8269d762c1b..029d77939c3 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -5393,7 +5393,7 @@ bool PeerConnection::UseCandidate(const IceCandidateInterface* candidate) { SetIceConnectionState(PeerConnectionInterface::kIceConnectionChecking); } // TODO(bemasc): If state is Completed, go back to Connected. - } else if (error.message()) { + } else { RTC_LOG(LS_WARNING) << error.message(); } return true; diff --git a/rtc_base/BUILD.gn b/rtc_base/BUILD.gn index 009671c7823..8a65e30fcb1 100644 --- a/rtc_base/BUILD.gn +++ b/rtc_base/BUILD.gn @@ -689,6 +689,9 @@ rtc_source_set("rtc_json") { "json.cc", "json.h", ] + deps = [ + ":stringutils", + ] all_dependent_configs = [ "//third_party/jsoncpp:jsoncpp_config" ] if (rtc_build_json) { public_deps = [ diff --git a/rtc_base/checks.h b/rtc_base/checks.h index a65088d3fda..9de6d47573c 100644 --- a/rtc_base/checks.h +++ b/rtc_base/checks.h @@ -40,7 +40,6 @@ RTC_NORETURN void rtc_FatalMessage(const char* file, int line, const char* msg); #ifdef __cplusplus // C++ version. -#include // no-presubmit-check TODO(webrtc:8982) #include #include "rtc_base/numerics/safe_compare.h" diff --git a/rtc_base/json.cc b/rtc_base/json.cc index cc40bd678de..f7716ab221d 100644 --- a/rtc_base/json.cc +++ b/rtc_base/json.cc @@ -14,25 +14,23 @@ #include #include -#include +#include "rtc_base/stringencode.h" namespace rtc { bool GetStringFromJson(const Json::Value& in, std::string* out) { if (!in.isString()) { - std::ostringstream s; if (in.isBool()) { - s << std::boolalpha << in.asBool(); + *out = rtc::ToString(in.asBool()); } else if (in.isInt()) { - s << in.asInt(); + *out = rtc::ToString(in.asInt()); } else if (in.isUInt()) { - s << in.asUInt(); + *out = rtc::ToString(in.asUInt()); } else if (in.isDouble()) { - s << in.asDouble(); + *out = rtc::ToString(in.asDouble()); } else { return false; } - *out = s.str(); } else { *out = in.asString(); } diff --git a/rtc_base/logging.cc b/rtc_base/logging.cc index ca03d8ea6ea..129a8974397 100644 --- a/rtc_base/logging.cc +++ b/rtc_base/logging.cc @@ -31,8 +31,6 @@ static const int kMaxLogLineSize = 1024 - 60; #include #include -#include -#include #include #include "rtc_base/criticalsection.h" diff --git a/rtc_base/logging.h b/rtc_base/logging.h index 4f74dcfbcec..1bd0c7205e6 100644 --- a/rtc_base/logging.h +++ b/rtc_base/logging.h @@ -47,7 +47,7 @@ #include #include -#include +#include // no-presubmit-check TODO(webrtc:8982) #include #include diff --git a/rtc_base/socketaddress.h b/rtc_base/socketaddress.h index 7095be189db..bff8e76495e 100644 --- a/rtc_base/socketaddress.h +++ b/rtc_base/socketaddress.h @@ -11,7 +11,6 @@ #ifndef RTC_BASE_SOCKETADDRESS_H_ #define RTC_BASE_SOCKETADDRESS_H_ -#include #include #ifdef UNIT_TEST #include // no-presubmit-check TODO(webrtc:8982) diff --git a/rtc_base/stringencode.h b/rtc_base/stringencode.h index f1f802d60b3..5d436dfd57b 100644 --- a/rtc_base/stringencode.h +++ b/rtc_base/stringencode.h @@ -11,7 +11,6 @@ #ifndef RTC_BASE_STRINGENCODE_H_ #define RTC_BASE_STRINGENCODE_H_ -#include #include #include diff --git a/test/fuzzers/rtp_packet_fuzzer.cc b/test/fuzzers/rtp_packet_fuzzer.cc index ef9bf2608ec..8bf8e74aba9 100644 --- a/test/fuzzers/rtp_packet_fuzzer.cc +++ b/test/fuzzers/rtp_packet_fuzzer.cc @@ -8,6 +8,8 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include + #include "modules/rtp_rtcp/include/rtp_header_extension_map.h" #include "modules/rtp_rtcp/source/rtp_generic_frame_descriptor_extension.h" #include "modules/rtp_rtcp/source/rtp_header_extensions.h"