Fix for issue #13

src/tests.rs

@@ -5,8 +5,59 @@ mod toodee_tests {
     use alloc::boxed::Box;
     use alloc::vec;
     use alloc::vec::Vec;
+    use core::marker::PhantomData;
+    use core::fmt::Display;
 
     use crate::*;
+
+    struct DropDetector<V : Display> {
+        value : V,
+    }
+
+    impl<V : Display> DropDetector<V> {
+        fn new(value : V) -> Self {
+            DropDetector { value : value }
+        }
+    }
+
+    impl<V : Display> Drop for DropDetector<V> {
+        fn drop(&mut self) {
+            println!("Dropping {}", self.value);
+        }
+    }
+
+    // An iterator that panics.
+    // -----
+    struct PanickingIterator<V> {
+        phantom: PhantomData<V>
+    }
+
+    impl<V> PanickingIterator<V> {
+        fn new() -> Self {
+            PanickingIterator { phantom : PhantomData }
+        }
+    }
+
+    impl<V> Iterator for PanickingIterator<V> {
+        type Item = V;
+        fn next(&mut self) -> Option<Self::Item> { panic!("Iterator panicked"); }
+    }
+
+    impl<V> ExactSizeIterator for PanickingIterator<V> {
+        fn len(&self) -> usize { 1 }
+    }
+
+    struct IteratorWithWrongLength();
+
+    impl Iterator for IteratorWithWrongLength {
+        type Item = Box<u8>;
+
+        fn next(&mut self) -> Option<Self::Item> { None }
+    }
+
+    impl ExactSizeIterator for IteratorWithWrongLength {
+        fn len(&self) -> usize { 1 }
+    }
 
     #[test]
     fn default() {
@@ -435,6 +486,23 @@ mod toodee_tests {
         toodee.insert_row(1, tmp);
     }
 
+    #[test]
+    #[should_panic(expected = "Iterator panicked")]
+    fn insert_row_iterator_panic() {
+        let vec = vec![DropDetector::new(1), DropDetector::new(2), DropDetector::new(3)];
+        let mut toodee : TooDee<_> = TooDee::from_vec(1, 3, vec);
+        toodee.insert_row(0, PanickingIterator::new());
+    }
+
+    #[test]
+    #[should_panic(expected = "assertion failed")]
+    fn insert_row_bad_exact_size_iterator() {
+        let vec = vec![Box::<u8>::new(1)];
+        let mut toodee : TooDee<_> = TooDee::from_vec(1, 1, vec);
+        toodee.insert_row(1, IteratorWithWrongLength());
+        println!("{}", toodee[1][0]);
+    }
+
     #[test]
     fn insert_col_1_0() {
         let mut toodee : TooDee<u32> = TooDee::from_vec(4, 1, (0u32..4).collect());

src/toodee.rs

@@ -671,14 +671,32 @@ impl<T> TooDee<T> {
 
         let start = index * self.num_cols;
         let len = self.data.len();
+
         unsafe {
+
+            // Prevent duplicate (or any) drops on the portion of the array we are modifying.
+            // This is to safe-guard against a panic potentially caused by `iter.next()`.
+            // Alternative (less performant) approaches would be:
+            // - append the new row to the array and use `slice.rotate...()` to shuffle everything into place.
+            // - store the new row data in a temporary location before shifting the memory and inserting the row.
+            self.data.set_len(start);
+
             let mut p = self.data.as_mut_ptr().add(start);
             // shift everything to make space for the new row
             ptr::copy(p, p.add(self.num_cols), len - start);
-            for e in iter {
+
+            let mut elem_count = 0usize;
+            // Use `take()` to cap the number of elements processed because we cannot rely on
+            // then `len()` value of `ExactSizeIterator` in unsafe code.
+            for e in iter.take(self.num_cols) {
                 ptr::write(p, e);
                 p = p.add(1);
+                elem_count += 1;
             }
+
+            // abort if the iterator length was less than expected
+            assert_eq!(self.num_cols, elem_count, "unexpected iterator length");
+
             self.data.set_len(len + self.num_cols);
         }
 
@@ -810,10 +828,23 @@ impl<T> TooDee<T> {
         let new_len = old_len + self.num_rows;
         let suffix_len = self.num_cols - index;
         unsafe {
+
+            // Prevent duplicate (or any) drops on the array we are modifying.
+            // This is to safe-guard against a panic potentially caused by `iter.rev()`.
+            // Alternative (less performant) approaches would be:
+            // - append the new column to the array and use swapping to shuffle everything into place.
+            // - store the new column data in a temporary location before shifting the memory and inserting values.
+            self.data.set_len(0);
+
             let p = self.data.as_mut_ptr();
             let mut read_p = p.add(old_len);
             let mut write_p = p.add(new_len);
-            for e in iter.rev() {
+
+            let mut elem_count = 0usize;
+
+            // Use `take()` to cap the number of elements processed because we cannot rely on
+            // then `len()` value of `ExactSizeIterator` in unsafe code.
+            for e in iter.take(self.num_rows).rev() {
                 // shift suffix
                 read_p = read_p.sub(suffix_len);
                 write_p = write_p.sub(suffix_len);
@@ -825,7 +856,12 @@ impl<T> TooDee<T> {
                 read_p = read_p.sub(index);
                 write_p = write_p.sub(index);
                 ptr::copy(read_p, write_p, index);
+                elem_count += 1;
             }
+
+            // abort if the iterator length was less than expected
+            assert_eq!(self.num_rows, elem_count, "unexpected iterator length");
+
             self.data.set_len(new_len);
         }