std: maintain CStringArray null-termination even if Vec::push panics#155774
std: maintain CStringArray null-termination even if Vec::push panics#155774rust-bors[bot] merged 1 commit intorust-lang:mainfrom
CStringArray null-termination even if Vec::push panics#155774Conversation
rustbot
added
S-waiting-on-review
|
r? @jhpratt rustbot has assigned @jhpratt. Use Why was this reviewer chosen?The reviewer was selected based on:
|
| @@ -35,10 +35,12 @@ | |||
| /// Push an additional string to the array. | |||
| pub fn push(&mut self, item: CString) { | |||
| let argc = self.ptrs.len() - 1; | |||
There was a problem hiding this comment.
Out of curiosity, is it possible for self.ptrs.len() to be 0/should we be worried about underflow here?
There was a problem hiding this comment.
No, since there is always at least one element (the null terminator) in the array.
| /// Creates a new `CStringArray` with enough capacity to hold `capacity` | ||
| /// strings. | ||
| pub fn with_capacity(capacity: usize) -> Self { | ||
| let mut result = CStringArray { ptrs: Vec::with_capacity(capacity + 1) }; |
There was a problem hiding this comment.
Never mind, I see it right here. Okay cool, our CStringArray will always have something in ptrs when creating it.
However, this does make me wonder if we should be worried about if capacity = usize::MAX here.
There was a problem hiding this comment.
We don't, all uses are inside std and cannot overflow. And even if this did overflow that wouldn't be a problem since nothing depends on this capacity.
|
@bors r+ rollup |
rust-bors
Bot
added
S-waiting-on-bors
std: maintain `CStringArray` null-termination even if `Vec::push` panics Fixes rust-lang#155748 by performing the `push` of the new null terminator before overwriting the previous one.
|
⌛ Testing commit bab4983 with merge fbaefca... Workflow: https://github.com/rust-lang/rust/actions/runs/24952695616 |
std: maintain `CStringArray` null-termination even if `Vec::push` panics Fixes #155748 by performing the `push` of the new null terminator before overwriting the previous one.
|
@bors yield |
|
Auto build was cancelled. Cancelled workflows: The next pull request likely to be tested is #155813. |
5 participants
Fixes #155748 by performing the
pushof the new null terminator before overwriting the previous one.