Skip to content

Commit

Permalink
use NonNull in allocation APIs
Browse files Browse the repository at this point in the history
  • Loading branch information
folkertdev committed Jan 6, 2025
1 parent 6a8c364 commit f085060
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 43 deletions.
37 changes: 14 additions & 23 deletions zlib-rs/src/allocate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use core::{
alloc::Layout,
ffi::{c_uint, c_void},
marker::PhantomData,
ptr::NonNull,
};

#[cfg(feature = "rust-allocator")]
Expand Down Expand Up @@ -160,14 +161,6 @@ impl Allocator<'static> {
};
}

fn null_is_none<T>(ptr: *mut T) -> Option<*mut T> {
if ptr.is_null() {
None
} else {
Some(ptr)
}
}

impl Allocator<'_> {
pub fn allocate_layout(&self, layout: Layout) -> *mut c_void {
// Special case for the Rust `alloc` backed allocator
Expand Down Expand Up @@ -253,21 +246,21 @@ impl Allocator<'_> {
ptr
}

pub fn allocate_raw<T>(&self) -> Option<*mut T> {
null_is_none(self.allocate_layout(Layout::new::<T>()).cast())
pub fn allocate_raw<T>(&self) -> Option<NonNull<T>> {
NonNull::new(self.allocate_layout(Layout::new::<T>()).cast())
}

pub fn allocate_slice_raw<T>(&self, len: usize) -> Option<*mut T> {
null_is_none(self.allocate_layout(Layout::array::<T>(len).ok()?).cast())
pub fn allocate_slice_raw<T>(&self, len: usize) -> Option<NonNull<T>> {
NonNull::new(self.allocate_layout(Layout::array::<T>(len).ok()?).cast())
}

pub fn allocate_zeroed(&self, len: usize) -> Option<*mut u8> {
pub fn allocate_zeroed(&self, len: usize) -> Option<NonNull<u8>> {
#[cfg(feature = "rust-allocator")]
if self.zalloc == Allocator::RUST.zalloc {
// internally, we want to align allocations to 64 bytes (in part for SIMD reasons)
let layout = Layout::from_size_align(len, 64).unwrap();

return null_is_none(unsafe { std::alloc::System.alloc_zeroed(layout) });
return NonNull::new(unsafe { std::alloc::System.alloc_zeroed(layout) });
}

#[cfg(feature = "c-allocator")]
Expand All @@ -281,18 +274,16 @@ impl Allocator<'_> {

let ptr = alloc.allocate_layout(Layout::array::<u8>(len).ok().unwrap());

return null_is_none(ptr.cast());
return NonNull::new(ptr.cast());
}

// create the allocation (contents are uninitialized)
let ptr = self.allocate_layout(Layout::array::<u8>(len).ok().unwrap());

if ptr.is_null() {
return None;
}
let ptr = NonNull::new(ptr)?;

// zero all contents (thus initializing the buffer)
unsafe { core::ptr::write_bytes(ptr, 0, len) };
unsafe { core::ptr::write_bytes(ptr.as_ptr(), 0, len) };

Some(ptr.cast())
}
Expand Down Expand Up @@ -365,11 +356,11 @@ mod tests {
_marker: PhantomData,
};

let ptr = allocator.allocate_raw::<T>().unwrap();
let ptr = allocator.allocate_raw::<T>().unwrap().as_ptr();
assert_eq!(ptr as usize % core::mem::align_of::<T>(), 0);
unsafe { allocator.deallocate(ptr, 1) }

let ptr = allocator.allocate_slice_raw::<T>(10).unwrap();
let ptr = allocator.allocate_slice_raw::<T>(10).unwrap().as_ptr();
assert_eq!(ptr as usize % core::mem::align_of::<T>(), 0);
unsafe { allocator.deallocate(ptr, 10) }
}
Expand Down Expand Up @@ -424,11 +415,11 @@ mod tests {
return;
};

let slice = unsafe { core::slice::from_raw_parts_mut(buf, len) };
let slice = unsafe { core::slice::from_raw_parts_mut(buf.as_ptr(), len) };

assert_eq!(slice.iter().sum::<u8>(), 0);

unsafe { allocator.deallocate(buf, len) };
unsafe { allocator.deallocate(buf.as_ptr(), len) };
}

#[test]
Expand Down
26 changes: 16 additions & 10 deletions zlib-rs/src/deflate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,27 +285,29 @@ pub fn init(stream: &mut z_stream, config: DeflateConfig) -> ReturnCode {
pending.drop_in(&alloc);
}
if let Some(head) = head {
alloc.deallocate(head, 1)
alloc.deallocate(head.as_ptr(), 1)
}
if let Some(prev) = prev {
alloc.deallocate(prev, w_size)
alloc.deallocate(prev.as_ptr(), w_size)
}
if let Some(mut window) = window {
window.drop_in(&alloc);
}

alloc.deallocate(state_allocation, 1);
alloc.deallocate(state_allocation.as_ptr(), 1);
}

return ReturnCode::MemError;
}
};

// zero initialize the memory
let prev = prev.as_ptr(); // FIXME: write_bytes is stable for NonNull since 1.80.0
unsafe { prev.write_bytes(0, w_size) };
let prev = unsafe { WeakSliceMut::from_raw_parts_mut(prev, w_size) };

// zero out head's first element
let head = head.as_ptr(); // FIXME: write_bytes is stable for NonNull since 1.80.0
unsafe { head.write_bytes(0, 1) };
let head = unsafe { WeakArrayMut::<u16, HASH_SIZE>::from_ptr(head) };

Expand Down Expand Up @@ -371,8 +373,9 @@ pub fn init(stream: &mut z_stream, config: DeflateConfig) -> ReturnCode {
hash_calc_variant: HashCalcVariant::Standard,
};

unsafe { state_allocation.write(state) };
stream.state = state_allocation as *mut internal_state;
unsafe { state_allocation.as_ptr().write(state) }; // FIXME: write is stable for NonNull since 1.80.0

stream.state = state_allocation.as_ptr() as *mut internal_state;

let Some(stream) = (unsafe { DeflateStream::from_stream_mut(stream) }) else {
if cfg!(debug_assertions) {
Expand Down Expand Up @@ -593,28 +596,31 @@ pub fn copy<'a>(
pending.drop_in(alloc);
}
if let Some(head) = head {
alloc.deallocate(head, HASH_SIZE)
alloc.deallocate(head.as_ptr(), HASH_SIZE)
}
if let Some(prev) = prev {
alloc.deallocate(prev, source_state.w_size)
alloc.deallocate(prev.as_ptr(), source_state.w_size)
}
if let Some(mut window) = window {
window.drop_in(alloc);
}

alloc.deallocate(state_allocation, 1);
alloc.deallocate(state_allocation.as_ptr(), 1);
}

return ReturnCode::MemError;
}
};

let prev = unsafe {
let prev = prev.as_ptr();
prev.copy_from_nonoverlapping(source_state.prev.as_ptr(), source_state.prev.len());
WeakSliceMut::from_raw_parts_mut(prev, source_state.prev.len())
};

// FIXME: write_bytes is stable for NonNull since 1.80.0
let head = unsafe {
let head = head.as_ptr();
head.write_bytes(0, 1);
head.cast::<u16>().write(source_state.head.as_slice()[0]);
WeakArrayMut::from_ptr(head)
Expand Down Expand Up @@ -667,11 +673,11 @@ pub fn copy<'a>(
};

// write the cloned state into state_ptr
unsafe { state_allocation.write(dest_state) };
unsafe { state_allocation.as_ptr().write(dest_state) }; // FIXME: write is stable for NonNull since 1.80.0

// insert the state_ptr into `dest`
let field_ptr = unsafe { core::ptr::addr_of_mut!((*dest.as_mut_ptr()).state) };
unsafe { core::ptr::write(field_ptr as *mut *mut State, state_allocation) };
unsafe { core::ptr::write(field_ptr as *mut *mut State, state_allocation.as_ptr()) };

// update the gzhead field (it contains a mutable reference so we need to be careful
let field_ptr = unsafe { core::ptr::addr_of_mut!((*dest.as_mut_ptr()).state.gzhead) };
Expand Down
2 changes: 1 addition & 1 deletion zlib-rs/src/deflate/pending.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl<'a> Pending<'a> {
pub(crate) fn new_in(alloc: &Allocator<'a>, len: usize) -> Option<Self> {
let ptr = alloc.allocate_slice_raw::<MaybeUninit<u8>>(len)?;
// SAFETY: freshly allocated buffer
let buf = unsafe { WeakSliceMut::from_raw_parts_mut(ptr, len) };
let buf = unsafe { WeakSliceMut::from_raw_parts_mut(ptr.as_ptr(), len) };

Some(Self {
buf,
Expand Down
2 changes: 1 addition & 1 deletion zlib-rs/src/deflate/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ impl<'a> Window<'a> {
let len = 2 * ((1 << window_bits) + Self::padding());
let ptr = alloc.allocate_zeroed(len)?;
// SAFETY: freshly allocated buffer
let buf = unsafe { WeakSliceMut::from_raw_parts_mut(ptr, len) };
let buf = unsafe { WeakSliceMut::from_raw_parts_mut(ptr.as_ptr(), len) };

Some(Self { buf, window_bits })
}
Expand Down
11 changes: 6 additions & 5 deletions zlib-rs/src/inflate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2121,8 +2121,9 @@ pub fn init(stream: &mut z_stream, config: InflateConfig) -> ReturnCode {
return ReturnCode::MemError;
};

unsafe { state_allocation.write(state) };
stream.state = state_allocation as *mut internal_state;
// FIXME: write is stable for NonNull since 1.80.0
unsafe { state_allocation.as_ptr().write(state) };
stream.state = state_allocation.as_ptr() as *mut internal_state;

// SAFETY: we've correctly initialized the stream to be an InflateStream
let ret = if let Some(stream) = unsafe { InflateStream::from_stream_mut(stream) } {
Expand Down Expand Up @@ -2470,19 +2471,19 @@ pub unsafe fn copy<'a>(
if !state.window.is_empty() {
let Some(window) = state.window.clone_in(&source.alloc) else {
// SAFETY: state_allocation is not used again.
source.alloc.deallocate(state_allocation, 1);
source.alloc.deallocate(state_allocation.as_ptr(), 1);
return ReturnCode::MemError;
};

copy.window = window;
}

// write the cloned state into state_ptr
unsafe { state_allocation.write(copy) };
unsafe { state_allocation.as_ptr().write(copy) }; // FIXME: write is stable for NonNull since 1.80.0

// insert the state_ptr into `dest`
let field_ptr = unsafe { core::ptr::addr_of_mut!((*dest.as_mut_ptr()).state) };
unsafe { core::ptr::write(field_ptr as *mut *mut State, state_allocation) };
unsafe { core::ptr::write(field_ptr as *mut *mut State, state_allocation.as_ptr()) };

// update the writer; it cannot be cloned so we need to use some shennanigans
let field_ptr = unsafe { core::ptr::addr_of_mut!((*dest.as_mut_ptr()).state.writer) };
Expand Down
4 changes: 2 additions & 2 deletions zlib-rs/src/inflate/window.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ impl<'a> Window<'a> {
let ptr = alloc.allocate_zeroed(len)?;

Some(Self {
buf: unsafe { WeakSliceMut::from_raw_parts_mut(ptr, len) },
buf: unsafe { WeakSliceMut::from_raw_parts_mut(ptr.as_ptr(), len) },
have: 0,
next: 0,
})
Expand All @@ -161,7 +161,7 @@ impl<'a> Window<'a> {
let ptr = alloc.allocate_zeroed(len)?;

Some(Self {
buf: unsafe { WeakSliceMut::from_raw_parts_mut(ptr, len) },
buf: unsafe { WeakSliceMut::from_raw_parts_mut(ptr.as_ptr(), len) },
have: self.have,
next: self.next,
})
Expand Down
2 changes: 1 addition & 1 deletion zlib-rs/src/read_buf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ impl<'a> ReadBuf<'a> {
let ptr = alloc.allocate_zeroed(len)?;

// safety: all elements are now initialized
let buf = unsafe { WeakSliceMut::from_raw_parts_mut(ptr, len) };
let buf = unsafe { WeakSliceMut::from_raw_parts_mut(ptr.as_ptr(), len) };

Some(Self { buf, filled: 0 })
}
Expand Down

0 comments on commit f085060

Please sign in to comment.