From eb4a3140fdc6b35f55260ecdb6f8c74a15beaba3 Mon Sep 17 00:00:00 2001
From: Tommi <tommi@webrtc.org>
Date: Mon, 15 Jan 2024 21:06:05 +0100
Subject: [PATCH] Add ReadStringView() to ByteBufferReader

ReadStringView() is a simple alternative to ReadString() but doesn't
involve a heap allocation for a new std::string.
Using the new methods in one place to start with.

Bug: none
Change-Id: I1100c6d258ffb4c8a31a46ba88a7f8bff9cf35cb
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/332120
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41533}
---
 api/transport/stun.cc            |  4 ++--
 pc/sctp_utils_unittest.cc        |  8 ++++----
 rtc_base/byte_buffer.cc          |  8 ++++++++
 rtc_base/byte_buffer.h           |  3 +++
 rtc_base/byte_buffer_unittest.cc | 21 +++++++++++++++++++++
 5 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/api/transport/stun.cc b/api/transport/stun.cc
index c359fd359d..ca90515952 100644
--- a/api/transport/stun.cc
+++ b/api/transport/stun.cc
@@ -603,8 +603,8 @@ bool StunMessage::Read(ByteBufferReader* buf) {
     return false;
   }
 
-  std::string magic_cookie;
-  if (!buf->ReadString(&magic_cookie, kStunMagicCookieLength)) {
+  absl::string_view magic_cookie;
+  if (!buf->ReadStringView(&magic_cookie, kStunMagicCookieLength)) {
     return false;
   }
 
diff --git a/pc/sctp_utils_unittest.cc b/pc/sctp_utils_unittest.cc
index 9ef2068a05..64ad257983 100644
--- a/pc/sctp_utils_unittest.cc
+++ b/pc/sctp_utils_unittest.cc
@@ -72,11 +72,11 @@ class SctpUtilsTest : public ::testing::Test {
     EXPECT_EQ(label.size(), label_length);
     EXPECT_EQ(config.protocol.size(), protocol_length);
 
-    std::string label_output;
-    ASSERT_TRUE(buffer.ReadString(&label_output, label_length));
+    absl::string_view label_output;
+    ASSERT_TRUE(buffer.ReadStringView(&label_output, label_length));
     EXPECT_EQ(label, label_output);
-    std::string protocol_output;
-    ASSERT_TRUE(buffer.ReadString(&protocol_output, protocol_length));
+    absl::string_view protocol_output;
+    ASSERT_TRUE(buffer.ReadStringView(&protocol_output, protocol_length));
     EXPECT_EQ(config.protocol, protocol_output);
   }
 };
diff --git a/rtc_base/byte_buffer.cc b/rtc_base/byte_buffer.cc
index 121097bae9..5674d54e21 100644
--- a/rtc_base/byte_buffer.cc
+++ b/rtc_base/byte_buffer.cc
@@ -132,6 +132,14 @@ bool ByteBufferReader::ReadString(std::string* val, size_t len) {
   }
 }
 
+bool ByteBufferReader::ReadStringView(absl::string_view* val, size_t len) {
+  if (!val || len > Length())
+    return false;
+  *val = absl::string_view(reinterpret_cast<const char*>(bytes_ + start_), len);
+  start_ += len;
+  return true;
+}
+
 bool ByteBufferReader::ReadBytes(rtc::ArrayView<uint8_t> val) {
   if (val.size() == 0) {
     return true;
diff --git a/rtc_base/byte_buffer.h b/rtc_base/byte_buffer.h
index 6df285ae6d..1508bd6ead 100644
--- a/rtc_base/byte_buffer.h
+++ b/rtc_base/byte_buffer.h
@@ -178,6 +178,9 @@ class ByteBufferReader {
   // Appends next `len` bytes from the buffer to `val`. Returns false
   // if there is less than `len` bytes left.
   bool ReadString(std::string* val, size_t len);
+  // Same as `ReadString` except that the returned string_view will point into
+  // the internal buffer (no additional buffer allocation).
+  bool ReadStringView(absl::string_view* val, size_t len);
 
   // Moves current position `size` bytes forward. Returns false if
   // there is less than `size` bytes left in the buffer. Consume doesn't
diff --git a/rtc_base/byte_buffer_unittest.cc b/rtc_base/byte_buffer_unittest.cc
index 49e2e569f5..520845d40b 100644
--- a/rtc_base/byte_buffer_unittest.cc
+++ b/rtc_base/byte_buffer_unittest.cc
@@ -210,6 +210,27 @@ TEST(ByteBufferTest, TestReadWriteBuffer) {
   buffer.Clear();
 }
 
+TEST(ByteBufferTest, TestReadStringView) {
+  const absl::string_view tests[] = {"hello", " ", "string_view"};
+  std::string buffer;
+  for (const auto& test : tests)
+    buffer += test;
+
+  rtc::ArrayView<const uint8_t> bytes(
+      reinterpret_cast<const uint8_t*>(&buffer[0]), buffer.size());
+
+  ByteBufferReader read_buf(bytes);
+  size_t consumed = 0;
+  for (const auto& test : tests) {
+    absl::string_view sv;
+    EXPECT_TRUE(read_buf.ReadStringView(&sv, test.length()));
+    EXPECT_EQ(sv.compare(test), 0);
+    // The returned string view should point directly into the original string.
+    EXPECT_EQ(&sv[0], &buffer[0 + consumed]);
+    consumed += sv.size();
+  }
+}
+
 TEST(ByteBufferTest, TestReadWriteUVarint) {
   ByteBufferWriter write_buffer;
   size_t size = 0;