Skip to content

Commit 731e3e9

Browse files
committed
Fixed resize() to prevent capacity shrinking and copy all nodes
## Description Changed resize() to properly enforce capacity constraints and copy all nodes including gaps: 1. Changed assert from self.len() to self.capacity() to prevent resizing to a buffer smaller than current capacity 2. Changed loop from 0..self.len() to 0..self.capacity() to ensure all nodes are copied, not just the count in use The storage layout can have gaps like [VALID | VALID | INVALID | VALID] where deleted nodes remain in the buffer. The loop must iterate over capacity to copy all nodes including invalid ones. ## How This Was Tested Added two tests: - test_resize_prevents_capacity_shrink(): Verifies assert prevents shrinking - test_resize_copies_all_nodes_including_gaps(): Verifies all nodes copied All 49 tests passing. Cargo clippy and fmt passing.
1 parent 1045104 commit 731e3e9

File tree

1 file changed

+77
-2
lines changed
  • core/patina_internal_collections/src

1 file changed

+77
-2
lines changed

core/patina_internal_collections/src/node.rs

Lines changed: 77 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,7 @@ where
199199
)
200200
};
201201

202-
assert!(buffer.len() >= self.len());
202+
assert!(buffer.len() >= self.capacity());
203203

204204
// When current capacity is 0, we just need to copy the data and build the available list
205205
if self.capacity() == 0 {
@@ -210,7 +210,7 @@ where
210210
}
211211

212212
// Copy the data from the old buffer to the new buffer. Update the pointers to the new buffer
213-
for i in 0..self.len() {
213+
for i in 0..self.capacity() {
214214
let old = &self.data[i];
215215

216216
buffer[i].data = old.data;
@@ -780,4 +780,79 @@ mod tests {
780780
assert_eq!(node2.right_ptr(), r1.as_mut_ptr());
781781
assert_eq!(r1.parent_ptr(), node2.as_mut_ptr());
782782
}
783+
784+
#[test]
785+
#[should_panic(expected = "assertion failed: buffer.len() >= self.capacity()")]
786+
fn test_resize_prevents_capacity_shrink() {
787+
// Verify that resize() prevents shrinking capacity
788+
const INITIAL_SIZE: usize = 10;
789+
let mut initial_memory = [0; INITIAL_SIZE * node_size::<usize>()];
790+
let mut storage = Storage::<usize>::with_capacity(&mut initial_memory);
791+
792+
// Add some nodes
793+
storage.add(100).unwrap();
794+
storage.add(200).unwrap();
795+
storage.add(300).unwrap();
796+
797+
// Now storage has capacity=10, length=3
798+
assert_eq!(storage.capacity(), 10);
799+
assert_eq!(storage.len(), 3);
800+
801+
// Try to resize to smaller capacity (5 < 10)
802+
// This should panic because we're shrinking capacity
803+
const SMALLER_SIZE: usize = 5;
804+
let mut smaller_memory = [0; SMALLER_SIZE * node_size::<usize>()];
805+
storage.resize(&mut smaller_memory); // Should panic here
806+
}
807+
808+
#[test]
809+
fn test_resize_copies_all_nodes_including_gaps() {
810+
// Test that resize copies ALL nodes (capacity), not just len() nodes
811+
// Buffer layout: [VALID | VALID | INVALID | VALID | INVALID]
812+
const INITIAL_SIZE: usize = 10;
813+
let mut initial_memory = [0; INITIAL_SIZE * node_size::<usize>()];
814+
let mut storage = Storage::<usize>::with_capacity(&mut initial_memory);
815+
816+
// Add 5 nodes at indices 0-4
817+
storage.add(100).unwrap(); // idx 0
818+
storage.add(200).unwrap(); // idx 1
819+
storage.add(300).unwrap(); // idx 2
820+
storage.add(400).unwrap(); // idx 3
821+
storage.add(500).unwrap(); // idx 4
822+
823+
// Delete nodes at indices 2 and 3 to create gaps
824+
let node2_ptr = storage.get_mut(2).unwrap().as_mut_ptr();
825+
let node3_ptr = storage.get_mut(3).unwrap().as_mut_ptr();
826+
storage.delete(node2_ptr);
827+
storage.delete(node3_ptr);
828+
829+
// Now add nodes that will use higher indices
830+
storage.add(600).unwrap(); // Reuses idx 3
831+
storage.add(700).unwrap(); // Reuses idx 2
832+
storage.add(800).unwrap(); // idx 5
833+
storage.add(900).unwrap(); // idx 6
834+
835+
// Storage now has: capacity=10, len=7
836+
// Valid nodes spread across indices with gaps
837+
assert_eq!(storage.capacity(), 10);
838+
assert_eq!(storage.len(), 7);
839+
840+
// Resize to larger capacity - should copy ALL nodes including invalid ones
841+
const LARGER_SIZE: usize = 20;
842+
let mut larger_memory = [0; LARGER_SIZE * node_size::<usize>()];
843+
storage.resize(&mut larger_memory);
844+
845+
// Verify all 7 nodes are still accessible
846+
assert_eq!(storage.len(), 7);
847+
assert_eq!(storage.capacity(), 20);
848+
849+
// Verify we can access all nodes
850+
assert!(storage.get(0).is_some());
851+
assert!(storage.get(1).is_some());
852+
assert!(storage.get(2).is_some());
853+
assert!(storage.get(3).is_some());
854+
assert!(storage.get(4).is_some());
855+
assert!(storage.get(5).is_some());
856+
assert!(storage.get(6).is_some());
857+
}
783858
}

0 commit comments

Comments
 (0)