Skip to content

Commit 8aad330

Browse files
committed
[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
1 parent 313f8a2 commit 8aad330

File tree

3 files changed

+34
-45
lines changed

3 files changed

+34
-45
lines changed

libc/src/__support/CPP/StringView.h

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -107,19 +107,14 @@ class StringView {
107107

108108
// Moves the start of the view forward by n characters.
109109
// The behavior is undefined if n > size().
110-
StringView remove_prefix(size_t N) const {
111-
if (N >= Len)
112-
return StringView();
113-
return StringView(Data + N, Len - N);
110+
void remove_prefix(size_t N) {
111+
Len -= N;
112+
Data += N;
114113
}
115114

116115
// Moves the end of the view back by n characters.
117116
// The behavior is undefined if n > size().
118-
StringView remove_suffix(size_t N) const {
119-
if (N >= Len)
120-
return StringView();
121-
return StringView(Data, Len - N);
122-
}
117+
void remove_suffix(size_t N) { Len -= N; }
123118

124119
// An equivalent method is not available in std::string_view.
125120
StringView trim(const char C) const {

libc/src/stdlib/getenv.cpp

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@ LLVM_LIBC_FUNCTION(char *, getenv, (const char *name)) {
3232
if (cur[env_var_name.size()] != '=')
3333
continue;
3434

35-
return const_cast<char *>(
36-
cur.remove_prefix(env_var_name.size() + 1).data());
35+
// Remove the name and the equals sign.
36+
cur.remove_prefix(env_var_name.size() + 1);
37+
// We know that data is null terminated, so this is safe.
38+
return const_cast<char *>(cur.data());
3739
}
3840

3941
return nullptr;

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

Lines changed: 26 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -78,43 +78,35 @@ TEST(LlvmLibcStringViewTest, endsWith) {
7878
}
7979

8080
TEST(LlvmLibcStringViewTest, RemovePrefix) {
81-
StringView v("123456789");
82-
83-
auto p = v.remove_prefix(0);
84-
ASSERT_EQ(p.size(), size_t(9));
85-
ASSERT_TRUE(p.equals(StringView("123456789")));
86-
87-
p = v.remove_prefix(4);
88-
ASSERT_EQ(p.size(), size_t(5));
89-
ASSERT_TRUE(p.equals(StringView("56789")));
90-
91-
p = v.remove_prefix(9);
92-
ASSERT_EQ(p.size(), size_t(0));
93-
ASSERT_TRUE(p.data() == nullptr);
94-
95-
p = v.remove_prefix(10);
96-
ASSERT_EQ(p.size(), size_t(0));
97-
ASSERT_TRUE(p.data() == nullptr);
81+
StringView a("123456789");
82+
a.remove_prefix(0);
83+
ASSERT_EQ(a.size(), size_t(9));
84+
ASSERT_TRUE(a.equals(StringView("123456789")));
85+
86+
StringView b("123456789");
87+
b.remove_prefix(4);
88+
ASSERT_EQ(b.size(), size_t(5));
89+
ASSERT_TRUE(b.equals(StringView("56789")));
90+
91+
StringView c("123456789");
92+
c.remove_prefix(9);
93+
ASSERT_EQ(c.size(), size_t(0));
9894
}
9995

10096
TEST(LlvmLibcStringViewTest, RemoveSuffix) {
101-
StringView v("123456789");
102-
103-
auto p = v.remove_suffix(0);
104-
ASSERT_EQ(p.size(), size_t(9));
105-
ASSERT_TRUE(p.equals(StringView("123456789")));
106-
107-
p = v.remove_suffix(4);
108-
ASSERT_EQ(p.size(), size_t(5));
109-
ASSERT_TRUE(p.equals(StringView("12345")));
110-
111-
p = v.remove_suffix(9);
112-
ASSERT_EQ(p.size(), size_t(0));
113-
ASSERT_TRUE(p.data() == nullptr);
114-
115-
p = v.remove_suffix(10);
116-
ASSERT_EQ(p.size(), size_t(0));
117-
ASSERT_TRUE(p.data() == nullptr);
97+
StringView a("123456789");
98+
a.remove_suffix(0);
99+
ASSERT_EQ(a.size(), size_t(9));
100+
ASSERT_TRUE(a.equals(StringView("123456789")));
101+
102+
StringView b("123456789");
103+
b.remove_suffix(4);
104+
ASSERT_EQ(b.size(), size_t(5));
105+
ASSERT_TRUE(b.equals(StringView("12345")));
106+
107+
StringView c("123456789");
108+
c.remove_suffix(9);
109+
ASSERT_EQ(c.size(), size_t(0));
118110
}
119111

120112
TEST(LlvmLibcStringViewTest, TrimSingleChar) {

0 commit comments

Comments
 (0)