[libcxx-commits] [PATCH] D119628: [libc++] Don't return alloc_size - 1 in __recommend to add 1 when allocating

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone requested changes to this revision.
Quuxplusone added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/string:2261-2264
     size_type __cap = __old_cap < __ms / 2 - __alignment ?
                           __recommend(_VSTD::max(__old_cap + __delta_cap, 2 * __old_cap)) :
                           __ms - 1;
+    pointer __p = __alloc_traits::allocate(__alloc(), __cap);
----------------
(1) I'm still very unsure that this is correct. @philnik, can you maybe make up a cheat-sheet that shows which of these numbers will include +1 for the null terminator and which won't?
- `x = capacity()`
- `__set_long_cap(x)`
- `x = __recommend(...)`
- `__grow_by(x, ...)`
- `__grow_by_and_replace(x, ...)`

Ideally, in the end, they'd all use the same meaning for "capacity" — which means it should //not// include the null terminator, because `capacity()` doesn't include the null terminator.

(2) On this diff in particular, aren't you accidentally failing to +1 the capacity when `(__old_cap < __ms / 2 - __alignment)`? Please investigate **and add a regression test for what you find.**


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119628/new/

https://reviews.llvm.org/D119628