From 590b21d43389df918764c1ba566002011c7fb92e Mon Sep 17 00:00:00 2001 From: Vinzenz Schroeter Date: Tue, 15 Oct 2024 22:23:52 +0200 Subject: [PATCH] use NonNull as return type in C API --- crates/servicepoint_binding_c/src/bitmap.rs | 28 +++-- crates/servicepoint_binding_c/src/bitvec.rs | 24 ++-- .../src/brightness_grid.rs | 27 +++-- .../servicepoint_binding_c/src/byte_slice.rs | 4 +- crates/servicepoint_binding_c/src/command.rs | 105 ++++++++---------- .../servicepoint_binding_c/src/cp437_grid.rs | 28 +++-- crates/servicepoint_binding_c/src/packet.rs | 17 ++- 7 files changed, 105 insertions(+), 128 deletions(-) diff --git a/crates/servicepoint_binding_c/src/bitmap.rs b/crates/servicepoint_binding_c/src/bitmap.rs index 0c4271c..b327fd2 100644 --- a/crates/servicepoint_binding_c/src/bitmap.rs +++ b/crates/servicepoint_binding_c/src/bitmap.rs @@ -2,6 +2,7 @@ //! //! prefix `sp_bitmap_` +use std::ptr::NonNull; use servicepoint::{DataRef, Grid}; use crate::byte_slice::SPByteSlice; @@ -41,12 +42,11 @@ pub struct SPBitmap(pub(crate) servicepoint::Bitmap); pub unsafe extern "C" fn sp_bitmap_new( width: usize, height: usize, -) -> *mut SPBitmap { - let result = Box::into_raw(Box::new(SPBitmap(servicepoint::Bitmap::new( +) -> NonNull { + let result = Box::new(SPBitmap(servicepoint::Bitmap::new( width, height, - )))); - assert!(!result.is_null()); - result + ))); + NonNull::from(Box::leak(result)) } /// Loads a [SPBitmap] with the specified dimensions from the provided data. @@ -77,14 +77,13 @@ pub unsafe extern "C" fn sp_bitmap_load( height: usize, data: *const u8, data_length: usize, -) -> *mut SPBitmap { +) -> NonNull { assert!(!data.is_null()); let data = std::slice::from_raw_parts(data, data_length); - let result = Box::into_raw(Box::new(SPBitmap(servicepoint::Bitmap::load( + let result = Box::new(SPBitmap(servicepoint::Bitmap::load( width, height, data, - )))); - assert!(!result.is_null()); - result + ))); + NonNull::from(Box::leak(result)) } /// Clones a [SPBitmap]. @@ -106,11 +105,10 @@ pub unsafe extern "C" fn sp_bitmap_load( #[no_mangle] pub unsafe extern "C" fn sp_bitmap_clone( bitmap: *const SPBitmap, -) -> *mut SPBitmap { +) -> NonNull { assert!(!bitmap.is_null()); - let result = Box::into_raw(Box::new(SPBitmap((*bitmap).0.clone()))); - assert!(!result.is_null()); - result + let result = Box::new(SPBitmap((*bitmap).0.clone())); + NonNull::from(Box::leak(result)) } /// Deallocates a [SPBitmap]. @@ -277,7 +275,7 @@ pub unsafe extern "C" fn sp_bitmap_unsafe_data_ref( assert!(!bitmap.is_null()); let data = (*bitmap).0.data_ref_mut(); SPByteSlice { - start: data.as_mut_ptr_range().start, + start: NonNull::new(data.as_mut_ptr_range().start).unwrap(), length: data.len(), } } diff --git a/crates/servicepoint_binding_c/src/bitvec.rs b/crates/servicepoint_binding_c/src/bitvec.rs index 1fe509b..7f2258a 100644 --- a/crates/servicepoint_binding_c/src/bitvec.rs +++ b/crates/servicepoint_binding_c/src/bitvec.rs @@ -2,6 +2,7 @@ //! //! prefix `sp_bitvec_` +use std::ptr::NonNull; use crate::SPByteSlice; use servicepoint::bitvec::prelude::{BitVec, Msb0}; @@ -52,10 +53,9 @@ impl Clone for SPBitVec { /// - the returned instance is freed in some way, either by using a consuming function or /// by explicitly calling `sp_bitvec_free`. #[no_mangle] -pub unsafe extern "C" fn sp_bitvec_new(size: usize) -> *mut SPBitVec { - let result = Box::into_raw(Box::new(SPBitVec(BitVec::repeat(false, size)))); - assert!(!result.is_null()); - result +pub unsafe extern "C" fn sp_bitvec_new(size: usize) -> NonNull { + let result = Box::new(SPBitVec(BitVec::repeat(false, size))); + NonNull::from(Box::leak(result)) } /// Interpret the data as a series of bits and load then into a new [SPBitVec] instance. @@ -78,12 +78,11 @@ pub unsafe extern "C" fn sp_bitvec_new(size: usize) -> *mut SPBitVec { pub unsafe extern "C" fn sp_bitvec_load( data: *const u8, data_length: usize, -) -> *mut SPBitVec { +) -> NonNull { assert!(!data.is_null()); let data = std::slice::from_raw_parts(data, data_length); - let result = Box::into_raw(Box::new(SPBitVec(BitVec::from_slice(data)))); - assert!(!result.is_null()); - result + let result = Box::new(SPBitVec(BitVec::from_slice(data))); + NonNull::from(Box::leak(result)) } /// Clones a [SPBitVec]. @@ -105,11 +104,10 @@ pub unsafe extern "C" fn sp_bitvec_load( #[no_mangle] pub unsafe extern "C" fn sp_bitvec_clone( bit_vec: *const SPBitVec, -) -> *mut SPBitVec { +) -> NonNull { assert!(!bit_vec.is_null()); - let result = Box::into_raw(Box::new((*bit_vec).clone())); - assert!(!result.is_null()); - result + let result = Box::new((*bit_vec).clone()); + NonNull::from(Box::leak(result)) } /// Deallocates a [SPBitVec]. @@ -278,7 +276,7 @@ pub unsafe extern "C" fn sp_bitvec_unsafe_data_ref( assert!(!bit_vec.is_null()); let data = (*bit_vec).0.as_raw_mut_slice(); SPByteSlice { - start: data.as_mut_ptr_range().start, + start: NonNull::new(data.as_mut_ptr_range().start).unwrap(), length: data.len(), } } diff --git a/crates/servicepoint_binding_c/src/brightness_grid.rs b/crates/servicepoint_binding_c/src/brightness_grid.rs index c1dc186..4a527fe 100644 --- a/crates/servicepoint_binding_c/src/brightness_grid.rs +++ b/crates/servicepoint_binding_c/src/brightness_grid.rs @@ -5,6 +5,7 @@ use crate::SPByteSlice; use servicepoint::{Brightness, DataRef, Grid, PrimitiveGrid}; use std::intrinsics::transmute; +use std::ptr::NonNull; /// A grid containing brightness values. /// @@ -38,12 +39,11 @@ pub struct SPBrightnessGrid(pub(crate) servicepoint::BrightnessGrid); pub unsafe extern "C" fn sp_brightness_grid_new( width: usize, height: usize, -) -> *mut SPBrightnessGrid { - let result = Box::into_raw(Box::new(SPBrightnessGrid( +) -> NonNull { + let result = Box::new(SPBrightnessGrid( servicepoint::BrightnessGrid::new(width, height), - ))); - assert!(!result.is_null()); - result + )); + NonNull::from(Box::leak(result)) } /// Loads a [SPBrightnessGrid] with the specified dimensions from the provided data. @@ -69,15 +69,14 @@ pub unsafe extern "C" fn sp_brightness_grid_load( height: usize, data: *const u8, data_length: usize, -) -> *mut SPBrightnessGrid { +) -> NonNull { assert!(!data.is_null()); let data = std::slice::from_raw_parts(data, data_length); let grid = PrimitiveGrid::load(width, height, data); let grid = servicepoint::BrightnessGrid::try_from(grid) .expect("invalid brightness value"); - let result = Box::into_raw(Box::new(SPBrightnessGrid(grid))); - assert!(!result.is_null()); - result + let result = Box::new(SPBrightnessGrid(grid)); + NonNull::from(Box::leak(result)) } /// Clones a [SPBrightnessGrid]. @@ -103,11 +102,10 @@ pub unsafe extern "C" fn sp_brightness_grid_load( #[no_mangle] pub unsafe extern "C" fn sp_brightness_grid_clone( brightness_grid: *const SPBrightnessGrid, -) -> *mut SPBrightnessGrid { +) -> NonNull { assert!(!brightness_grid.is_null()); - let result = Box::into_raw(Box::new((*brightness_grid).clone())); - assert!(!result.is_null()); - result + let result = Box::new((*brightness_grid).clone()); + NonNull::from(Box::leak(result)) } /// Deallocates a [SPBrightnessGrid]. @@ -305,9 +303,10 @@ pub unsafe extern "C" fn sp_brightness_grid_unsafe_data_ref( assert!(!brightness_grid.is_null()); assert_eq!(core::mem::size_of::(), 1); let data = (*brightness_grid).0.data_ref_mut(); + // this assumes more about the memory layout than rust guarantees. yikes! let data: &mut [u8] = transmute(data); SPByteSlice { - start: data.as_mut_ptr_range().start, + start: NonNull::new(data.as_mut_ptr_range().start).unwrap(), length: data.len(), } } diff --git a/crates/servicepoint_binding_c/src/byte_slice.rs b/crates/servicepoint_binding_c/src/byte_slice.rs index fbdda2a..678a5d7 100644 --- a/crates/servicepoint_binding_c/src/byte_slice.rs +++ b/crates/servicepoint_binding_c/src/byte_slice.rs @@ -1,5 +1,7 @@ //! FFI slice helper +use std::ptr::NonNull; + #[repr(C)] /// Represents a span of memory (`&mut [u8]` ) as a struct usable by C code. /// @@ -16,7 +18,7 @@ /// will try to free the memory of a potentially separate allocator. pub struct SPByteSlice { /// The start address of the memory - pub start: *mut u8, + pub start: NonNull, /// The amount of memory in bytes pub length: usize, } diff --git a/crates/servicepoint_binding_c/src/command.rs b/crates/servicepoint_binding_c/src/command.rs index db86551..a4ebc1a 100644 --- a/crates/servicepoint_binding_c/src/command.rs +++ b/crates/servicepoint_binding_c/src/command.rs @@ -2,7 +2,7 @@ //! //! prefix `sp_command_` -use std::ptr::null_mut; +use std::ptr::{null_mut, NonNull}; use servicepoint::{Brightness, Origin}; @@ -80,11 +80,10 @@ pub unsafe extern "C" fn sp_command_try_from_packet( #[no_mangle] pub unsafe extern "C" fn sp_command_clone( command: *const SPCommand, -) -> *mut SPCommand { +) -> NonNull { assert!(!command.is_null()); - let result = Box::into_raw(Box::new((*command).clone())); - assert!(!result.is_null()); - result + let result = Box::new((*command).clone()); + NonNull::from(Box::leak(result)) } /// Set all pixels to the off state. @@ -106,11 +105,9 @@ pub unsafe extern "C" fn sp_command_clone( /// - the returned [SPCommand] instance is freed in some way, either by using a consuming function or /// by explicitly calling `sp_command_free`. #[no_mangle] -pub unsafe extern "C" fn sp_command_clear() -> *mut SPCommand { - let result = - Box::into_raw(Box::new(SPCommand(servicepoint::Command::Clear))); - assert!(!result.is_null()); - result +pub unsafe extern "C" fn sp_command_clear() -> NonNull { + let result = Box::new(SPCommand(servicepoint::Command::Clear)); + NonNull::from(Box::leak(result)) } /// Kills the udp daemon on the display, which usually results in a restart. @@ -126,11 +123,9 @@ pub unsafe extern "C" fn sp_command_clear() -> *mut SPCommand { /// - the returned [SPCommand] instance is freed in some way, either by using a consuming function or /// by explicitly calling `sp_command_free`. #[no_mangle] -pub unsafe extern "C" fn sp_command_hard_reset() -> *mut SPCommand { - let result = - Box::into_raw(Box::new(SPCommand(servicepoint::Command::HardReset))); - assert!(!result.is_null()); - result +pub unsafe extern "C" fn sp_command_hard_reset() -> NonNull { + let result = Box::new(SPCommand(servicepoint::Command::HardReset)); + NonNull::from(Box::leak(result)) } /// A yet-to-be-tested command. @@ -144,11 +139,9 @@ pub unsafe extern "C" fn sp_command_hard_reset() -> *mut SPCommand { /// - the returned [SPCommand] instance is freed in some way, either by using a consuming function or /// by explicitly calling `sp_command_free`. #[no_mangle] -pub unsafe extern "C" fn sp_command_fade_out() -> *mut SPCommand { - let result = - Box::into_raw(Box::new(SPCommand(servicepoint::Command::FadeOut))); - assert!(!result.is_null()); - result +pub unsafe extern "C" fn sp_command_fade_out() -> NonNull { + let result = Box::new(SPCommand(servicepoint::Command::FadeOut)); + NonNull::from(Box::leak(result)) } /// Set the brightness of all tiles to the same value. @@ -168,14 +161,13 @@ pub unsafe extern "C" fn sp_command_fade_out() -> *mut SPCommand { #[no_mangle] pub unsafe extern "C" fn sp_command_brightness( brightness: u8, -) -> *mut SPCommand { +) -> NonNull { let brightness = Brightness::try_from(brightness).expect("invalid brightness"); - let result = Box::into_raw(Box::new(SPCommand( + let result = Box::new(SPCommand( servicepoint::Command::Brightness(brightness), - ))); - assert!(!result.is_null()); - result + )); + NonNull::from(Box::leak(result)) } /// Set the brightness of individual tiles in a rectangular area of the display. @@ -201,14 +193,13 @@ pub unsafe extern "C" fn sp_command_char_brightness( x: usize, y: usize, grid: *mut SPBrightnessGrid, -) -> *mut SPCommand { +) -> NonNull { assert!(!grid.is_null()); let byte_grid = *Box::from_raw(grid); - let result = Box::into_raw(Box::new(SPCommand( + let result = Box::new(SPCommand( servicepoint::Command::CharBrightness(Origin::new(x, y), byte_grid.0), - ))); - assert!(!result.is_null()); - result + )); + NonNull::from(Box::leak(result)) } /// Set pixel data starting at the pixel offset on screen. @@ -241,18 +232,17 @@ pub unsafe extern "C" fn sp_command_bitmap_linear( offset: usize, bit_vec: *mut SPBitVec, compression: SPCompressionCode, -) -> *mut SPCommand { +) -> NonNull { assert!(!bit_vec.is_null()); let bit_vec = *Box::from_raw(bit_vec); - let result = Box::into_raw(Box::new(SPCommand( + let result = Box::new(SPCommand( servicepoint::Command::BitmapLinear( offset, bit_vec.into(), compression.try_into().expect("invalid compression code"), ), - ))); - assert!(!result.is_null()); - result + )); + NonNull::from(Box::leak(result)) } /// Set pixel data according to an and-mask starting at the offset. @@ -285,18 +275,17 @@ pub unsafe extern "C" fn sp_command_bitmap_linear_and( offset: usize, bit_vec: *mut SPBitVec, compression: SPCompressionCode, -) -> *mut SPCommand { +) -> NonNull { assert!(!bit_vec.is_null()); let bit_vec = *Box::from_raw(bit_vec); - let result = Box::into_raw(Box::new(SPCommand( + let result = Box::new(SPCommand( servicepoint::Command::BitmapLinearAnd( offset, bit_vec.into(), compression.try_into().expect("invalid compression code"), ), - ))); - assert!(!result.is_null()); - result + )); + NonNull::from(Box::leak(result)) } /// Set pixel data according to an or-mask starting at the offset. @@ -329,18 +318,17 @@ pub unsafe extern "C" fn sp_command_bitmap_linear_or( offset: usize, bit_vec: *mut SPBitVec, compression: SPCompressionCode, -) -> *mut SPCommand { +) -> NonNull { assert!(!bit_vec.is_null()); let bit_vec = *Box::from_raw(bit_vec); - let result = Box::into_raw(Box::new(SPCommand( + let result = Box::new(SPCommand( servicepoint::Command::BitmapLinearOr( offset, bit_vec.into(), compression.try_into().expect("invalid compression code"), ), - ))); - assert!(!result.is_null()); - result + )); + NonNull::from(Box::leak(result)) } /// Set pixel data according to a xor-mask starting at the offset. @@ -373,18 +361,17 @@ pub unsafe extern "C" fn sp_command_bitmap_linear_xor( offset: usize, bit_vec: *mut SPBitVec, compression: SPCompressionCode, -) -> *mut SPCommand { +) -> NonNull { assert!(!bit_vec.is_null()); let bit_vec = *Box::from_raw(bit_vec); - let result = Box::into_raw(Box::new(SPCommand( + let result = Box::new(SPCommand( servicepoint::Command::BitmapLinearXor( offset, bit_vec.into(), compression.try_into().expect("invalid compression code"), ), - ))); - assert!(!result.is_null()); - result + )); + NonNull::from(Box::leak(result)) } /// Show text on the screen. @@ -410,14 +397,13 @@ pub unsafe extern "C" fn sp_command_cp437_data( x: usize, y: usize, grid: *mut SPCp437Grid, -) -> *mut SPCommand { +) -> NonNull { assert!(!grid.is_null()); let grid = *Box::from_raw(grid); - let result = Box::into_raw(Box::new(SPCommand( + let result = Box::new(SPCommand( servicepoint::Command::Cp437Data(Origin::new(x, y), grid.0), - ))); - assert!(!result.is_null()); - result + )); + NonNull::from(Box::leak(result)) } /// Sets a window of pixels to the specified values. @@ -446,10 +432,10 @@ pub unsafe extern "C" fn sp_command_bitmap_linear_win( y: usize, bitmap: *mut SPBitmap, compression_code: SPCompressionCode, -) -> *mut SPCommand { +) -> NonNull { assert!(!bitmap.is_null()); let byte_grid = (*Box::from_raw(bitmap)).0; - let result = Box::into_raw(Box::new(SPCommand( + let result = Box::new(SPCommand( servicepoint::Command::BitmapLinearWin( Origin::new(x, y), byte_grid, @@ -457,9 +443,8 @@ pub unsafe extern "C" fn sp_command_bitmap_linear_win( .try_into() .expect("invalid compression code"), ), - ))); - assert!(!result.is_null()); - result + )); + NonNull::from(Box::leak(result)) } /// Deallocates a [SPCommand]. diff --git a/crates/servicepoint_binding_c/src/cp437_grid.rs b/crates/servicepoint_binding_c/src/cp437_grid.rs index e458a1b..b940ae4 100644 --- a/crates/servicepoint_binding_c/src/cp437_grid.rs +++ b/crates/servicepoint_binding_c/src/cp437_grid.rs @@ -2,6 +2,7 @@ //! //! prefix `sp_cp437_grid_` +use std::ptr::NonNull; use crate::SPByteSlice; use servicepoint::{DataRef, Grid}; @@ -39,12 +40,11 @@ impl Clone for SPCp437Grid { pub unsafe extern "C" fn sp_cp437_grid_new( width: usize, height: usize, -) -> *mut SPCp437Grid { - let result = Box::into_raw(Box::new(SPCp437Grid( +) -> NonNull { + let result = Box::new(SPCp437Grid( servicepoint::Cp437Grid::new(width, height), - ))); - assert!(!result.is_null()); - result + )); + NonNull::from(Box::leak(result)) } /// Loads a [SPCp437Grid] with the specified dimensions from the provided data. @@ -70,14 +70,13 @@ pub unsafe extern "C" fn sp_cp437_grid_load( height: usize, data: *const u8, data_length: usize, -) -> *mut SPCp437Grid { +) -> NonNull { assert!(data.is_null()); let data = std::slice::from_raw_parts(data, data_length); - let result = Box::into_raw(Box::new(SPCp437Grid( + let result = Box::new(SPCp437Grid( servicepoint::Cp437Grid::load(width, height, data), - ))); - assert!(!result.is_null()); - result + )); + NonNull::from(Box::leak(result)) } /// Clones a [SPCp437Grid]. @@ -99,11 +98,10 @@ pub unsafe extern "C" fn sp_cp437_grid_load( #[no_mangle] pub unsafe extern "C" fn sp_cp437_grid_clone( cp437_grid: *const SPCp437Grid, -) -> *mut SPCp437Grid { +) -> NonNull { assert!(!cp437_grid.is_null()); - let result = Box::into_raw(Box::new((*cp437_grid).clone())); - assert!(!result.is_null()); - result + let result = Box::new((*cp437_grid).clone()); + NonNull::from(Box::leak(result)) } /// Deallocates a [SPCp437Grid]. @@ -278,7 +276,7 @@ pub unsafe extern "C" fn sp_cp437_grid_unsafe_data_ref( ) -> SPByteSlice { let data = (*cp437_grid).0.data_ref_mut(); SPByteSlice { - start: data.as_mut_ptr_range().start, + start: NonNull::new(data.as_mut_ptr_range().start).unwrap(), length: data.len(), } } diff --git a/crates/servicepoint_binding_c/src/packet.rs b/crates/servicepoint_binding_c/src/packet.rs index f2cffc4..e44c23f 100644 --- a/crates/servicepoint_binding_c/src/packet.rs +++ b/crates/servicepoint_binding_c/src/packet.rs @@ -2,7 +2,7 @@ //! //! prefix `sp_packet_` -use std::ptr::null_mut; +use std::ptr::{null_mut, NonNull}; use crate::SPCommand; @@ -29,13 +29,11 @@ pub struct SPPacket(pub(crate) servicepoint::packet::Packet); #[no_mangle] pub unsafe extern "C" fn sp_packet_from_command( command: *mut SPCommand, -) -> *mut SPPacket { +) -> NonNull { assert!(!command.is_null()); let command = *Box::from_raw(command); - let packet = SPPacket(command.0.into()); - let result = Box::into_raw(Box::new(packet)); - assert!(!result.is_null()); - result + let result = Box::new(SPPacket(command.0.into())); + NonNull::from(Box::leak(result)) } /// Tries to load a [SPPacket] from the passed array with the specified length. @@ -86,11 +84,10 @@ pub unsafe extern "C" fn sp_packet_try_load( #[no_mangle] pub unsafe extern "C" fn sp_packet_clone( packet: *const SPPacket, -) -> *mut SPPacket { +) -> NonNull { assert!(!packet.is_null()); - let result = Box::into_raw(Box::new(SPPacket((*packet).0.clone()))); - assert!(!result.is_null()); - result + let result = Box::new(SPPacket((*packet).0.clone())); + NonNull::from(Box::leak(result)) } /// Deallocates a [SPPacket].