mirror of
https://github.com/jart/cosmopolitan.git
synced 2025-01-31 03:27:39 +00:00
Fix some memory issues with ctl::string (#1201)
There were a few errors in how capacity and memory was being handled for small strings. The capacity errors meant that small strings would become big strings too soon, and the memory error introduced undefined behavior that was caught by CheckMemoryLeaks in our test file but only sometimes. The crucial change is in reserve: we only copy n bytes into p2, and then we manually set the null terminator instead of expecting it to have been there already. (E.g. it might not be there for an empty small string.) We also fix one other doozy in append when we were exactly at the small- to-big string boundary: we set the last byte (i.e., the remainder field) to 0, then decremented it, giving us size_t max. Whoops. We boneheadedly fix this by setting the 0 byte after we've fixed up the remainder, so it is at worst a no-op. Otherwise, capacity now works the same for small strings as it does with big strings: it's the amount of space available including the null byte. We test all of this with a new test that only gets included if our class under test is not std::string (presumably meaning it's ctl::string.) The test manually verifies that the small string optimization behaves how we expect. Since this test checks against std::string, we go ahead and include that other header from the STL. Also modifies the new test we introduced to also run on std::string, but it just does the append without expecting anything about how its data is stored. We also check that the string has the right value afterwards.
This commit is contained in:
parent
f3effcb703
commit
2ba6b0158f
3 changed files with 34 additions and 5 deletions
|
@ -99,7 +99,8 @@ string::reserve(size_t c2) noexcept
|
|||
if (!isbig()) {
|
||||
if (!(p2 = (char*)malloc(c2)))
|
||||
__builtin_trap();
|
||||
memcpy(p2, data(), size() + 1);
|
||||
memcpy(p2, data(), size());
|
||||
p2[size()] = 0;
|
||||
} else {
|
||||
if (!(p2 = (char*)realloc(big()->p, c2)))
|
||||
__builtin_trap();
|
||||
|
@ -134,18 +135,18 @@ string::append(char ch) noexcept
|
|||
if (ckd_add(&n2, size(), 2))
|
||||
__builtin_trap();
|
||||
if (n2 > capacity()) {
|
||||
size_t c2 = capacity() + 2;
|
||||
size_t c2 = capacity();
|
||||
if (ckd_add(&c2, c2, c2 >> 1))
|
||||
__builtin_trap();
|
||||
reserve(c2);
|
||||
}
|
||||
data()[size()] = ch;
|
||||
data()[size() + 1] = 0;
|
||||
if (isbig()) {
|
||||
++big()->n;
|
||||
} else {
|
||||
--small()->rem;
|
||||
}
|
||||
data()[size()] = 0;
|
||||
}
|
||||
|
||||
void
|
||||
|
|
|
@ -151,7 +151,7 @@ class string
|
|||
if (isbig() && big()->c <= __::sso_max)
|
||||
__builtin_trap();
|
||||
#endif
|
||||
return isbig() ? __::big_mask & big()->c : __::sso_max;
|
||||
return isbig() ? __::big_mask & big()->c : __::string_size;
|
||||
}
|
||||
|
||||
iterator begin() noexcept
|
||||
|
|
|
@ -18,6 +18,7 @@
|
|||
|
||||
#include "ctl/string.h"
|
||||
|
||||
#include <__type_traits/is_same.h>
|
||||
#include <__utility/move.h>
|
||||
|
||||
#include "libc/runtime/runtime.h"
|
||||
|
@ -312,7 +313,7 @@ main()
|
|||
s.append(" world");
|
||||
}
|
||||
if (s != "hello world world world world world world world world world "
|
||||
"world world") {
|
||||
"world world") {
|
||||
return 64;
|
||||
}
|
||||
}
|
||||
|
@ -353,6 +354,33 @@ main()
|
|||
return 78;
|
||||
}
|
||||
|
||||
{
|
||||
ctl::string s;
|
||||
#undef ctl
|
||||
if constexpr (std::is_same_v<ctl::string, decltype(s)>) {
|
||||
// tests the small-string optimization on ctl::string
|
||||
char* d = s.data();
|
||||
for (int i = 0; i < 23; ++i) {
|
||||
s.append("a");
|
||||
if (s.data() != d) {
|
||||
return 79 + i;
|
||||
}
|
||||
}
|
||||
s.append("a");
|
||||
if (s.data() == d) {
|
||||
return 103;
|
||||
}
|
||||
} else {
|
||||
// just check that append in a loop works
|
||||
for (int i = 0; i < 24; ++i) {
|
||||
s.append("a");
|
||||
}
|
||||
}
|
||||
if (s != "aaaaaaaaaaaaaaaaaaaaaaaa") {
|
||||
return 104;
|
||||
}
|
||||
}
|
||||
|
||||
CheckForMemoryLeaks();
|
||||
return 0;
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue