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

[jeffbailey]

Summary

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

[jeffbailey]

We found this bug while adapting the TZ variable parsing code to the cpp::StringView class.

libc/src/__support/CPP/StringView.h

public:
   inline bool operator<=(StringView Other) const { return compare(Other) != 1; }
   inline bool operator>(StringView Other) const { return compare(Other) == 1; }
   inline bool operator>=(StringView Other) const {
     return compare(Other) != -1;
   }
 
   // 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 {
     StringView Copy = *this;
     while (Copy.starts_with(C))
       Copy = Copy.drop_front();
     while (Copy.ends_with(C))
       Copy = Copy.drop_back();

libc/src/stdlib/getenv.cpp

LLVM_LIBC_FUNCTION(char *, getenv, (const char *name)) {
   for (char **env = env_ptr; *env != nullptr; env++) {
     __llvm_libc::cpp::StringView cur(*env);
     if (!cur.starts_with(env_var_name))
       continue;
 
     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;
 }
 
 } // namespace __llvm_libc

libc/test/src/__support/CPP/stringview_test.cpp

TEST(LlvmLibcStringViewTest, endsWith) {
   ASSERT_FALSE(v.ends_with('1'));
   ASSERT_FALSE(v.ends_with(StringView("123")));
   ASSERT_FALSE(v.ends_with(StringView("abd")));
   ASSERT_FALSE(v.ends_with(StringView("aaa")));
   ASSERT_FALSE(v.ends_with(StringView("abcde")));
 }
 
 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) {
   StringView v("     123456789   ");
   auto t = v.trim(' ');
   ASSERT_EQ(t.size(), size_t(9));
   ASSERT_TRUE(t.equals(StringView("123456789")));