[libc-commits] [libc] 8aad330 - [libc] Fix API for remove_{prefix, suffix}

[Jeff Bailey via libc-commits]

Author: Jeff Bailey
Date: 2022-07-18T14:40:09Z
New Revision: 8aad330eebc0b9cfd8dd00e8ed692cb89e7577df

URL: https://github.com/llvm/llvm-project/commit/8aad330eebc0b9cfd8dd00e8ed692cb89e7577df
DIFF: https://github.com/llvm/llvm-project/commit/8aad330eebc0b9cfd8dd00e8ed692cb89e7577df.diff

LOG: [libc] Fix API for remove_{prefix, suffix}

The API in StringView.h for remove_prefix was incorrect and was returning a
new StringView rather than just altering the view.

As part of this, also removed some of the safety features.  The comment
correctly noted that the behaviour is undefined in some cases,
but the code and test cases checked for that.

One caller was relying on the old behaviour, so fixed it and added some
comments.

Tested:
check-libc
llvmlibc
libc-loader-tests

Reviewed By: gchatelet

Differential Revision: https://reviews.llvm.org/D129950

Added: 
    

Modified: 
    libc/src/__support/CPP/StringView.h
    libc/src/stdlib/getenv.cpp
    libc/test/src/__support/CPP/stringview_test.cpp

Removed: 
    


################################################################################
diff  --git a/libc/src/__support/CPP/StringView.h b/libc/src/__support/CPP/StringView.h
index 0f2b728724de..e8387aeedd6a 100644
--- a/libc/src/__support/CPP/StringView.h
+++ b/libc/src/__support/CPP/StringView.h
@@ -107,19 +107,14 @@ class StringView {
 
   // Moves the start of the view forward by n characters.
   // The behavior is undefined if n > size().
-  StringView remove_prefix(size_t N) const {
-    if (N >= Len)
-      return StringView();
-    return StringView(Data + N, Len - N);
+  void remove_prefix(size_t N) {
+    Len -= N;
+    Data += N;
   }
 
   // Moves the end of the view back by n characters.
   // The behavior is undefined if n > size().
-  StringView remove_suffix(size_t N) const {
-    if (N >= Len)
-      return StringView();
-    return StringView(Data, Len - N);
-  }
+  void remove_suffix(size_t N) { Len -= N; }
 
   // An equivalent method is not available in std::string_view.
   StringView trim(const char C) const {

diff  --git a/libc/src/stdlib/getenv.cpp b/libc/src/stdlib/getenv.cpp
index 2492d63d6d50..fa8e1fd9e423 100644
--- a/libc/src/stdlib/getenv.cpp
+++ b/libc/src/stdlib/getenv.cpp
@@ -32,8 +32,10 @@ LLVM_LIBC_FUNCTION(char *, getenv, (const char *name)) {
     if (cur[env_var_name.size()] != '=')
       continue;
 
-    return const_cast<char *>(
-        cur.remove_prefix(env_var_name.size() + 1).data());
+    // Remove the name and the equals sign.
+    cur.remove_prefix(env_var_name.size() + 1);
+    // We know that data is null terminated, so this is safe.
+    return const_cast<char *>(cur.data());
   }
 
   return nullptr;

diff  --git a/libc/test/src/__support/CPP/stringview_test.cpp b/libc/test/src/__support/CPP/stringview_test.cpp
index 5ea7b9fcfa1f..a20116b29eda 100644
--- a/libc/test/src/__support/CPP/stringview_test.cpp
+++ b/libc/test/src/__support/CPP/stringview_test.cpp
@@ -78,43 +78,35 @@ TEST(LlvmLibcStringViewTest, endsWith) {
 }
 
 TEST(LlvmLibcStringViewTest, RemovePrefix) {
-  StringView v("123456789");
-
-  auto p = v.remove_prefix(0);
-  ASSERT_EQ(p.size(), size_t(9));
-  ASSERT_TRUE(p.equals(StringView("123456789")));
-
-  p = v.remove_prefix(4);
-  ASSERT_EQ(p.size(), size_t(5));
-  ASSERT_TRUE(p.equals(StringView("56789")));
-
-  p = v.remove_prefix(9);
-  ASSERT_EQ(p.size(), size_t(0));
-  ASSERT_TRUE(p.data() == nullptr);
-
-  p = v.remove_prefix(10);
-  ASSERT_EQ(p.size(), size_t(0));
-  ASSERT_TRUE(p.data() == nullptr);
+  StringView a("123456789");
+  a.remove_prefix(0);
+  ASSERT_EQ(a.size(), size_t(9));
+  ASSERT_TRUE(a.equals(StringView("123456789")));
+
+  StringView b("123456789");
+  b.remove_prefix(4);
+  ASSERT_EQ(b.size(), size_t(5));
+  ASSERT_TRUE(b.equals(StringView("56789")));
+
+  StringView c("123456789");
+  c.remove_prefix(9);
+  ASSERT_EQ(c.size(), size_t(0));
 }
 
 TEST(LlvmLibcStringViewTest, RemoveSuffix) {
-  StringView v("123456789");
-
-  auto p = v.remove_suffix(0);
-  ASSERT_EQ(p.size(), size_t(9));
-  ASSERT_TRUE(p.equals(StringView("123456789")));
-
-  p = v.remove_suffix(4);
-  ASSERT_EQ(p.size(), size_t(5));
-  ASSERT_TRUE(p.equals(StringView("12345")));
-
-  p = v.remove_suffix(9);
-  ASSERT_EQ(p.size(), size_t(0));
-  ASSERT_TRUE(p.data() == nullptr);
-
-  p = v.remove_suffix(10);
-  ASSERT_EQ(p.size(), size_t(0));
-  ASSERT_TRUE(p.data() == nullptr);
+  StringView a("123456789");
+  a.remove_suffix(0);
+  ASSERT_EQ(a.size(), size_t(9));
+  ASSERT_TRUE(a.equals(StringView("123456789")));
+
+  StringView b("123456789");
+  b.remove_suffix(4);
+  ASSERT_EQ(b.size(), size_t(5));
+  ASSERT_TRUE(b.equals(StringView("12345")));
+
+  StringView c("123456789");
+  c.remove_suffix(9);
+  ASSERT_EQ(c.size(), size_t(0));
 }
 
 TEST(LlvmLibcStringViewTest, TrimSingleChar) {