Skip to content

Commit ae9bc12

Browse files
committed
some fixes
Signed-off-by: Adam Gutglick <adam@spiraldb.com>
1 parent 14ce14d commit ae9bc12

2 files changed

Lines changed: 41 additions & 17 deletions

File tree

vortex-buffer/src/bit/buf_mut.rs

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -650,6 +650,8 @@ impl FromIterator<bool> for BitBufferMut {
650650

651651
#[cfg(test)]
652652
mod tests {
653+
use rstest::rstest;
654+
653655
use crate::BufferMut;
654656
use crate::bit::buf_mut::BitBufferMut;
655657
use crate::bitbuffer;
@@ -924,6 +926,20 @@ mod tests {
924926
assert!(frozen.value(7));
925927
}
926928

929+
#[test]
930+
fn append_after_clear_reads_back_false() {
931+
// `clear` must not leave stale set bits behind: `append_false` and `append_buffer`
932+
// rely on bits beyond `len` being zero.
933+
let mut bools = BitBufferMut::new_set(16);
934+
bools.clear();
935+
bools.append_false();
936+
bools.append_buffer(&crate::BitBuffer::new_unset(8));
937+
938+
let bools = bools.freeze();
939+
assert_eq!(bools.len(), 9);
940+
assert_eq!(bools.true_count(), 0);
941+
}
942+
927943
#[test]
928944
fn test_append_buffer_after_truncate() {
929945
// Truncating leaves stale set bits in the last partial byte; an append after that
@@ -942,22 +958,27 @@ mod tests {
942958
}
943959
}
944960

945-
#[test]
946-
fn test_append_buffer_misaligned_long() {
947-
// Force mismatched source/destination bit offsets across many words.
961+
#[rstest]
962+
#[case::both_aligned(0, 0)]
963+
#[case::dst_unaligned(3, 0)]
964+
#[case::src_unaligned(0, 5)]
965+
#[case::mismatched(3, 5)]
966+
#[case::equal_nonzero(5, 5)]
967+
fn test_append_buffer_long(#[case] dst_prefix: usize, #[case] src_start: usize) {
968+
// Exercise every alignment combination across many words.
948969
let source = crate::BitBuffer::from_iter((0..301).map(|i| i % 3 == 0));
949-
let source = source.slice(5..301);
970+
let source = source.slice(src_start..301);
950971

951972
let mut dest = BitBufferMut::with_capacity(512);
952-
dest.append_n(true, 3);
973+
dest.append_n(true, dst_prefix);
953974
dest.append_buffer(&source);
954975

955-
assert_eq!(dest.len(), 3 + source.len());
956-
for i in 0..3 {
976+
assert_eq!(dest.len(), dst_prefix + source.len());
977+
for i in 0..dst_prefix {
957978
assert!(dest.value(i), "prefix bit {i}");
958979
}
959980
for i in 0..source.len() {
960-
assert_eq!(dest.value(3 + i), source.value(i), "bit {i}");
981+
assert_eq!(dest.value(dst_prefix + i), source.value(i), "bit {i}");
961982
}
962983
}
963984

vortex-buffer/src/buffer.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,17 +32,20 @@ pub struct Buffer<T> {
3232
pub(crate) _marker: PhantomData<T>,
3333
}
3434

35-
/// Zero-length backing memory for empty buffers, aligned to [`Alignment::MAX`] so it satisfies
36-
/// any valid alignment without allocating.
37-
#[repr(align(32768))]
38-
struct EmptyBacking([u8; 0]);
39-
40-
static EMPTY_BACKING: EmptyBacking = EmptyBacking([]);
35+
/// Zero-length backing for empty buffers, "aligned" to [`Alignment::MAX`] so it satisfies any
36+
/// valid alignment without allocating. A zero-length slice never reads memory, so it may use a
37+
/// dangling pointer as long as it is non-null and aligned.
38+
const EMPTY_BACKING: &[u8] = {
39+
let addr = 1usize << 15;
40+
assert!(Alignment::MAX.is_offset_aligned(addr));
41+
// SAFETY: the pointer is non-null and aligned, and the slice is zero-length.
42+
unsafe { std::slice::from_raw_parts(std::ptr::without_provenance(addr), 0) }
43+
};
4144

4245
impl<T> Default for Buffer<T> {
4346
fn default() -> Self {
4447
Self {
45-
bytes: Bytes::from_static(&EMPTY_BACKING.0),
48+
bytes: Bytes::from_static(EMPTY_BACKING),
4649
length: 0,
4750
alignment: Alignment::of::<T>(),
4851
_marker: PhantomData,
@@ -113,7 +116,7 @@ impl<T> Buffer<T> {
113116

114117
/// Create a new empty `ByteBuffer` with the provided alignment.
115118
///
116-
/// This does not allocate: empty buffers are backed by a shared static allocation that is
119+
/// This does not allocate: empty buffers are backed by a zero-length `Bytes` that is
117120
/// aligned to [`Alignment::MAX`].
118121
pub fn empty_aligned(alignment: Alignment) -> Self {
119122
if !alignment.is_aligned_to(Alignment::of::<T>()) {
@@ -124,7 +127,7 @@ impl<T> Buffer<T> {
124127
);
125128
}
126129
Self {
127-
bytes: Bytes::from_static(&EMPTY_BACKING.0),
130+
bytes: Bytes::from_static(EMPTY_BACKING),
128131
length: 0,
129132
alignment,
130133
_marker: PhantomData,

0 commit comments

Comments
 (0)