-
Notifications
You must be signed in to change notification settings - Fork 19
Open
Description
The get_unchecked_mut call within drop_in_place sometimes panics.
When the capacity of the extended vector is not enough to hold all extra values extend_ex tries to drop the rest. However the calculation on what this rest is, is wrong. It should use the length of the iter variable and not the capacity (as it is currently doing).
The following patch fixes this:
diff --git a/src/trait_impls.rs b/src/trait_impls.rs
index 8e912f2..b127f6d 100644
--- a/src/trait_impls.rs
+++ b/src/trait_impls.rs
@@ -215,16 +215,21 @@ impl<T, const N1: usize, const N2: usize> ExtendEx<T, StaticVec<T, N1>> for Stat
.as_ptr()
.copy_to_nonoverlapping(self.mut_ptr_at_unchecked(old_length), added_length);
self.set_len(old_length + added_length);
- // Wrap the values in a MaybeUninit to inhibit their destructors (if any),
- // then manually drop any excess ones. This is the same kind of "trick" as
- // is used in `new_from_array`, as you may or may not have noticed.
- let mut forgotten = MaybeUninit::new(iter);
- ptr::drop_in_place(
- forgotten
- .assume_init_mut()
- .as_mut_slice()
- .get_unchecked_mut(N1.min(N2)..N1),
- );
+
+ let drop_range = N1.min(N2)..iter.len().try_into().unwrap();
+ if !drop_range.is_empty() {
+ // Wrap the values in a MaybeUninit to inhibit their destructors (if any),
+ // then manually drop any excess ones. This is the same kind of "trick" as
+ // is used in `new_from_array`, as you may or may not have noticed.
+ let mut forgotten = MaybeUninit::new(iter);
+
+ ptr::drop_in_place(
+ forgotten
+ .assume_init_mut()
+ .as_mut_slice()
+ .get_unchecked_mut(drop_range),
+ );
+ }
}
}
And here is a test to reproduce the error
#[cfg(test)]
mod test {
use crate::StaticVec;
#[test]
fn test_extend_drop_no_panic() {
let mut a = StaticVec::<u64, 6>::new();
let mut b = StaticVec::<u64, 6>::new();
a.extend_from_slice(&[1, 2]);
b.extend_from_slice(&[3, 4, 5]);
// This panics
a.extend(b);
assert_eq!(a, [1, 2, 3, 4, 5]);
}
}
I would send a PR but #60 is blocking any real changes.
Metadata
Metadata
Assignees
Labels
No labels