From f483b5a2d5e3deea2b43a11b86cf65a20968bcb5 Mon Sep 17 00:00:00 2001 From: Vinzenz Schroeter Date: Sat, 12 Apr 2025 18:05:01 +0200 Subject: [PATCH] unify heap allocation handling --- include/servicepoint.h | 16 ++++----- src/bitmap.rs | 16 ++++----- src/bitvec.rs | 20 ++++------- src/brightness_grid.rs | 12 +++---- src/char_grid.rs | 12 +++---- src/command.rs | 78 +++++++++++++++++++----------------------- src/connection.rs | 18 +++++----- src/cp437_grid.rs | 12 +++---- src/lib.rs | 13 +++++++ src/packet.rs | 14 ++++---- 10 files changed, 101 insertions(+), 110 deletions(-) diff --git a/include/servicepoint.h b/include/servicepoint.h index 8f7ca75..9edeaf9 100644 --- a/include/servicepoint.h +++ b/include/servicepoint.h @@ -851,13 +851,6 @@ TypedCommand *sp_command_bitvec(size_t offset, CompressionCode compression, BinaryOperation operation); -/** - * Set the brightness of all tiles to the same value. - * - * Returns: a new [servicepoint::Command::Brightness] instance. - */ -TypedCommand */*notnull*/ sp_command_brightness(Brightness brightness); - /** * Set the brightness of individual tiles in a rectangular area of the display. * @@ -865,7 +858,7 @@ TypedCommand */*notnull*/ sp_command_brightness(Brightness brightness); * * Returns: a new [servicepoint::Command::CharBrightness] instance. */ -TypedCommand */*notnull*/ sp_command_char_brightness(size_t x, +TypedCommand */*notnull*/ sp_command_brightness_grid(size_t x, size_t y, BrightnessGrid */*notnull*/ grid); @@ -932,6 +925,13 @@ TypedCommand */*notnull*/ sp_command_fade_out(void); */ void sp_command_free(TypedCommand */*notnull*/ command); +/** + * Set the brightness of all tiles to the same value. + * + * Returns: a new [servicepoint::Command::Brightness] instance. + */ +TypedCommand */*notnull*/ sp_command_global_brightness(Brightness brightness); + /** * Kills the udp daemon on the display, which usually results in a restart. * diff --git a/src/bitmap.rs b/src/bitmap.rs index d1d8522..c166881 100644 --- a/src/bitmap.rs +++ b/src/bitmap.rs @@ -1,8 +1,8 @@ +use crate::byte_slice::ByteSlice; +use crate::{heap_drop, heap_move, heap_move_nonnull}; use servicepoint::{Bitmap, DataRef, Grid}; use std::ptr::NonNull; -use crate::byte_slice::ByteSlice; - /// Creates a new [Bitmap] with the specified dimensions. /// /// # Arguments @@ -32,7 +32,7 @@ pub unsafe extern "C" fn sp_bitmap_new( height: usize, ) -> *mut Bitmap { if let Some(bitmap) = Bitmap::new(width, height) { - Box::leak(Box::new(bitmap)) + heap_move(bitmap) } else { std::ptr::null_mut() } @@ -43,8 +43,7 @@ pub unsafe extern "C" fn sp_bitmap_new( /// returns: [Bitmap] initialized to all pixels off. #[no_mangle] pub unsafe extern "C" fn sp_bitmap_new_max_sized() -> NonNull { - let result = Box::new(Bitmap::max_sized()); - NonNull::from(Box::leak(result)) + heap_move_nonnull(Bitmap::max_sized()) } /// Loads a [Bitmap] with the specified dimensions from the provided data. @@ -63,7 +62,7 @@ pub unsafe extern "C" fn sp_bitmap_load( ) -> *mut Bitmap { let data = unsafe { data.as_slice() }; if let Ok(bitmap) = Bitmap::load(width, height, data) { - Box::leak(Box::new(bitmap)) + heap_move(bitmap) } else { std::ptr::null_mut() } @@ -74,14 +73,13 @@ pub unsafe extern "C" fn sp_bitmap_load( pub unsafe extern "C" fn sp_bitmap_clone( bitmap: NonNull, ) -> NonNull { - let result = Box::new(unsafe { bitmap.as_ref().clone() }); - NonNull::from(Box::leak(result)) + heap_move_nonnull(unsafe { bitmap.as_ref().clone() }) } /// Deallocates a [Bitmap]. #[no_mangle] pub unsafe extern "C" fn sp_bitmap_free(bitmap: NonNull) { - _ = unsafe { Box::from_raw(bitmap.as_ptr()) }; + unsafe { heap_drop(bitmap) } } /// Gets the current value at the specified position in the [Bitmap]. diff --git a/src/bitvec.rs b/src/bitvec.rs index f87423d..e5ce720 100644 --- a/src/bitvec.rs +++ b/src/bitvec.rs @@ -1,6 +1,6 @@ -use crate::ByteSlice; -use std::ptr::NonNull; +use crate::{heap_drop, heap_move_nonnull, ByteSlice}; use servicepoint::BitVecU8Msb0; +use std::ptr::NonNull; /// A vector of bits /// @@ -31,21 +31,16 @@ impl Clone for SPBitVec { /// - when `size` is not divisible by 8. #[no_mangle] pub unsafe extern "C" fn sp_bitvec_new(size: usize) -> NonNull { - let result = - Box::new(SPBitVec(BitVecU8Msb0::repeat(false, size))); - NonNull::from(Box::leak(result)) + heap_move_nonnull(SPBitVec(BitVecU8Msb0::repeat(false, size))) } /// Interpret the data as a series of bits and load then into a new [SPBitVec] instance. /// /// returns: [SPBitVec] instance containing data. #[no_mangle] -pub unsafe extern "C" fn sp_bitvec_load( - data: ByteSlice, -) -> NonNull { +pub unsafe extern "C" fn sp_bitvec_load(data: ByteSlice) -> NonNull { let data = unsafe { data.as_slice() }; - let result = Box::new(SPBitVec(BitVecU8Msb0::from_slice(data))); - NonNull::from(Box::leak(result)) + heap_move_nonnull(SPBitVec(BitVecU8Msb0::from_slice(data))) } /// Clones a [SPBitVec]. @@ -53,14 +48,13 @@ pub unsafe extern "C" fn sp_bitvec_load( pub unsafe extern "C" fn sp_bitvec_clone( bit_vec: NonNull, ) -> NonNull { - let result = Box::new(unsafe { bit_vec.as_ref().clone() }); - NonNull::from(Box::leak(result)) + heap_move_nonnull(unsafe { bit_vec.as_ref().clone() }) } /// Deallocates a [SPBitVec]. #[no_mangle] pub unsafe extern "C" fn sp_bitvec_free(bit_vec: NonNull) { - _ = unsafe { Box::from_raw(bit_vec.as_ptr()) }; + unsafe { heap_drop(bit_vec) } } /// Gets the value of a bit from the [SPBitVec]. diff --git a/src/brightness_grid.rs b/src/brightness_grid.rs index 5cd7a16..a283d0a 100644 --- a/src/brightness_grid.rs +++ b/src/brightness_grid.rs @@ -1,4 +1,4 @@ -use crate::ByteSlice; +use crate::{heap_drop, heap_move, heap_move_nonnull, ByteSlice}; use servicepoint::{Brightness, BrightnessGrid, ByteGrid, DataRef, Grid}; use std::mem::transmute; use std::ptr::NonNull; @@ -25,8 +25,7 @@ pub unsafe extern "C" fn sp_brightness_grid_new( width: usize, height: usize, ) -> NonNull { - let result = Box::new(BrightnessGrid::new(width, height)); - NonNull::from(Box::leak(result)) + heap_move_nonnull(BrightnessGrid::new(width, height)) } /// Loads a [BrightnessGrid] with the specified dimensions from the provided data. @@ -44,7 +43,7 @@ pub unsafe extern "C" fn sp_brightness_grid_load( Some(grid) => grid, }; if let Ok(grid) = BrightnessGrid::try_from(grid) { - Box::leak(Box::new(grid)) + heap_move(grid) } else { std::ptr::null_mut() } @@ -55,8 +54,7 @@ pub unsafe extern "C" fn sp_brightness_grid_load( pub unsafe extern "C" fn sp_brightness_grid_clone( brightness_grid: NonNull, ) -> NonNull { - let result = Box::new(unsafe { brightness_grid.as_ref().clone() }); - NonNull::from(Box::leak(result)) + heap_move_nonnull(unsafe { brightness_grid.as_ref().clone() }) } /// Deallocates a [BrightnessGrid]. @@ -64,7 +62,7 @@ pub unsafe extern "C" fn sp_brightness_grid_clone( pub unsafe extern "C" fn sp_brightness_grid_free( brightness_grid: NonNull, ) { - _ = unsafe { Box::from_raw(brightness_grid.as_ptr()) }; + unsafe { heap_drop(brightness_grid) } } /// Gets the current value at the specified position. diff --git a/src/char_grid.rs b/src/char_grid.rs index a287d70..cdcc4b5 100644 --- a/src/char_grid.rs +++ b/src/char_grid.rs @@ -1,4 +1,4 @@ -use crate::ByteSlice; +use crate::{heap_drop, heap_move, heap_move_nonnull, ByteSlice}; use servicepoint::{CharGrid, Grid}; use std::ptr::NonNull; @@ -19,8 +19,7 @@ pub unsafe extern "C" fn sp_char_grid_new( width: usize, height: usize, ) -> NonNull { - let result = Box::new(CharGrid::new(width, height)); - NonNull::from(Box::leak(result)) + heap_move_nonnull(CharGrid::new(width, height)) } /// Loads a [CharGrid] with the specified dimensions from the provided data. @@ -34,7 +33,7 @@ pub unsafe extern "C" fn sp_char_grid_load( ) -> *mut CharGrid { let data = unsafe { data.as_slice() }; if let Ok(grid) = CharGrid::load_utf8(width, height, data.to_vec()) { - Box::leak(Box::new(grid)) + heap_move(grid) } else { std::ptr::null_mut() } @@ -45,14 +44,13 @@ pub unsafe extern "C" fn sp_char_grid_load( pub unsafe extern "C" fn sp_char_grid_clone( char_grid: NonNull, ) -> NonNull { - let result = unsafe { char_grid.as_ref().clone() }; - NonNull::from(Box::leak(Box::new(result))) + heap_move_nonnull(unsafe { char_grid.as_ref().clone() }) } /// Deallocates a [CharGrid]. #[no_mangle] pub unsafe extern "C" fn sp_char_grid_free(char_grid: NonNull) { - _ = unsafe { Box::from_raw(char_grid.as_ptr()) }; + unsafe { heap_drop(char_grid) } } /// Returns the current value at the specified position. diff --git a/src/command.rs b/src/command.rs index 80a2b22..32d5acd 100644 --- a/src/command.rs +++ b/src/command.rs @@ -1,5 +1,8 @@ -use crate::SPBitVec; -use servicepoint::{BinaryOperation, Bitmap, Brightness, BrightnessGrid, CharGrid, CompressionCode, Cp437Grid, GlobalBrightnessCommand, Packet, TypedCommand}; +use crate::{heap_drop, heap_move, heap_move_nonnull, SPBitVec}; +use servicepoint::{ + BinaryOperation, Bitmap, Brightness, BrightnessGrid, CharGrid, + CompressionCode, Cp437Grid, GlobalBrightnessCommand, Packet, TypedCommand, +}; use std::ptr::NonNull; /// Tries to turn a [Packet] into a [TypedCommand]. @@ -14,7 +17,7 @@ pub unsafe extern "C" fn sp_command_try_from_packet( let packet = *unsafe { Box::from_raw(packet.as_ptr()) }; match servicepoint::TypedCommand::try_from(packet) { Err(_) => std::ptr::null_mut(), - Ok(command) => Box::into_raw(Box::new(command)), + Ok(command) => heap_move(command), } } @@ -25,8 +28,7 @@ pub unsafe extern "C" fn sp_command_try_from_packet( pub unsafe extern "C" fn sp_command_clone( command: NonNull, ) -> NonNull { - let result = Box::new(unsafe { command.as_ref().clone() }); - NonNull::from(Box::leak(result)) + heap_move_nonnull(unsafe { command.as_ref().clone() }) } /// Set all pixels to the off state. @@ -42,8 +44,7 @@ pub unsafe extern "C" fn sp_command_clone( /// ``` #[no_mangle] pub unsafe extern "C" fn sp_command_clear() -> NonNull { - let result = Box::new(servicepoint::ClearCommand.into()); - NonNull::from(Box::leak(result)) + heap_move_nonnull(servicepoint::ClearCommand.into()) } /// Kills the udp daemon on the display, which usually results in a restart. @@ -53,8 +54,7 @@ pub unsafe extern "C" fn sp_command_clear() -> NonNull { /// Returns: a new [servicepoint::Command::HardReset] instance. #[no_mangle] pub unsafe extern "C" fn sp_command_hard_reset() -> NonNull { - let result = Box::new(servicepoint::HardResetCommand.into()); - NonNull::from(Box::leak(result)) + heap_move_nonnull(servicepoint::HardResetCommand.into()) } /// A yet-to-be-tested command. @@ -62,19 +62,17 @@ pub unsafe extern "C" fn sp_command_hard_reset() -> NonNull { /// Returns: a new [servicepoint::Command::FadeOut] instance. #[no_mangle] pub unsafe extern "C" fn sp_command_fade_out() -> NonNull { - let result = Box::new(servicepoint::FadeOutCommand.into()); - NonNull::from(Box::leak(result)) + heap_move_nonnull(servicepoint::FadeOutCommand.into()) } /// Set the brightness of all tiles to the same value. /// /// Returns: a new [servicepoint::Command::Brightness] instance. #[no_mangle] -pub unsafe extern "C" fn sp_command_brightness( +pub unsafe extern "C" fn sp_command_global_brightness( brightness: Brightness, ) -> NonNull { - let result = Box::new(GlobalBrightnessCommand::from(brightness).into()); - NonNull::from(Box::leak(result)) + heap_move_nonnull(GlobalBrightnessCommand::from(brightness).into()) } /// Set the brightness of individual tiles in a rectangular area of the display. @@ -83,20 +81,18 @@ pub unsafe extern "C" fn sp_command_brightness( /// /// Returns: a new [servicepoint::Command::CharBrightness] instance. #[no_mangle] -pub unsafe extern "C" fn sp_command_char_brightness( +pub unsafe extern "C" fn sp_command_brightness_grid( x: usize, y: usize, grid: NonNull, ) -> NonNull { let grid = unsafe { *Box::from_raw(grid.as_ptr()) }; - let result = Box::new( - servicepoint::BrightnessGridCommand { - origin: servicepoint::Origin::new(x, y), - grid, - } - .into(), - ); - NonNull::from(Box::leak(result)) + let result = servicepoint::BrightnessGridCommand { + origin: servicepoint::Origin::new(x, y), + grid, + } + .into(); + heap_move_nonnull(result) } /// Set pixel data starting at the pixel offset on screen. @@ -129,8 +125,8 @@ pub unsafe extern "C" fn sp_command_bitvec( bitvec: bit_vec.0, compression, } - .into(); - Box::leak(Box::new(command)) + .into(); + heap_move(command) } /// Show codepage 437 encoded text on the screen. @@ -145,14 +141,12 @@ pub unsafe extern "C" fn sp_command_cp437_grid( grid: NonNull, ) -> NonNull { let grid = *unsafe { Box::from_raw(grid.as_ptr()) }; - let result = Box::new( - servicepoint::Cp437GridCommand { - origin: servicepoint::Origin::new(x, y), - grid, - } - .into(), - ); - NonNull::from(Box::leak(result)) + let result = servicepoint::Cp437GridCommand { + origin: servicepoint::Origin::new(x, y), + grid, + } + .into(); + heap_move_nonnull(result) } /// Show UTF-8 encoded text on the screen. @@ -167,14 +161,12 @@ pub unsafe extern "C" fn sp_command_char_grid( grid: NonNull, ) -> NonNull { let grid = unsafe { *Box::from_raw(grid.as_ptr()) }; - let result = Box::new( - servicepoint::CharGridCommand { - origin: servicepoint::Origin::new(x, y), - grid, - } - .into(), - ); - NonNull::from(Box::leak(result)) + let result = servicepoint::CharGridCommand { + origin: servicepoint::Origin::new(x, y), + grid, + } + .into(); + heap_move_nonnull(result) } /// Sets a window of pixels to the specified values. @@ -200,7 +192,7 @@ pub unsafe extern "C" fn sp_command_bitmap( compression, } .into(); - Box::leak(Box::new(command)) + heap_move(command) } /// Deallocates a [TypedCommand]. @@ -213,5 +205,5 @@ pub unsafe extern "C" fn sp_command_bitmap( /// ``` #[no_mangle] pub unsafe extern "C" fn sp_command_free(command: NonNull) { - _ = unsafe { Box::from_raw(command.as_ptr()) }; + unsafe { heap_drop(command) } } diff --git a/src/connection.rs b/src/connection.rs index b49225d..7b29e29 100644 --- a/src/connection.rs +++ b/src/connection.rs @@ -1,3 +1,4 @@ +use crate::{heap_drop, heap_move}; use servicepoint::{Connection, Packet, TypedCommand, UdpConnection}; use std::ffi::{c_char, CStr}; use std::net::{Ipv4Addr, SocketAddrV4}; @@ -26,7 +27,7 @@ pub unsafe extern "C" fn sp_udp_open( Ok(value) => value, }; - Box::into_raw(Box::new(connection)) + heap_move(connection) } /// Creates a new instance of [UdpConnection]. @@ -42,15 +43,18 @@ pub unsafe extern "C" fn sp_udp_open( /// ``` #[no_mangle] pub unsafe extern "C" fn sp_udp_open_ipv4( - ip1: u8, ip2: u8, ip3: u8, ip4: u8, + ip1: u8, + ip2: u8, + ip3: u8, + ip4: u8, port: u16, ) -> *mut UdpConnection { - let addr = SocketAddrV4::new(Ipv4Addr::from( [ip1, ip2, ip3, ip4]), port); + let addr = SocketAddrV4::new(Ipv4Addr::from([ip1, ip2, ip3, ip4]), port); let connection = match UdpConnection::open(addr) { Err(_) => return std::ptr::null_mut(), Ok(value) => value, }; - Box::into_raw(Box::new(connection)) + heap_move(connection) } /// Sends a [Packet] to the display using the [UdpConnection]. @@ -90,8 +94,6 @@ pub unsafe extern "C" fn sp_udp_send_command( /// Closes and deallocates a [UdpConnection]. #[no_mangle] -pub unsafe extern "C" fn sp_udp_free( - connection: NonNull, -) { - _ = unsafe { Box::from_raw(connection.as_ptr()) }; +pub unsafe extern "C" fn sp_udp_free(connection: NonNull) { + unsafe { heap_drop(connection) } } diff --git a/src/cp437_grid.rs b/src/cp437_grid.rs index 3904354..e22a0ed 100644 --- a/src/cp437_grid.rs +++ b/src/cp437_grid.rs @@ -1,4 +1,4 @@ -use crate::ByteSlice; +use crate::{heap_drop, heap_move, heap_move_nonnull, ByteSlice}; use servicepoint::{Cp437Grid, DataRef, Grid}; use std::ptr::NonNull; @@ -10,8 +10,7 @@ pub unsafe extern "C" fn sp_cp437_grid_new( width: usize, height: usize, ) -> NonNull { - let result = Box::new(Cp437Grid::new(width, height)); - NonNull::from(Box::leak(result)) + heap_move_nonnull(Cp437Grid::new(width, height)) } /// Loads a [Cp437Grid] with the specified dimensions from the provided data. @@ -24,7 +23,7 @@ pub unsafe extern "C" fn sp_cp437_grid_load( let data = unsafe { data.as_slice() }; let grid = Cp437Grid::load(width, height, data); if let Some(grid) = grid { - Box::leak(Box::new(grid)) + heap_move(grid) } else { std::ptr::null_mut() } @@ -35,14 +34,13 @@ pub unsafe extern "C" fn sp_cp437_grid_load( pub unsafe extern "C" fn sp_cp437_grid_clone( cp437_grid: NonNull, ) -> NonNull { - let result = Box::new(unsafe { cp437_grid.as_ref().clone() }); - NonNull::from(Box::leak(result)) + heap_move_nonnull(unsafe { cp437_grid.as_ref().clone() }) } /// Deallocates a [Cp437Grid]. #[no_mangle] pub unsafe extern "C" fn sp_cp437_grid_free(cp437_grid: NonNull) { - _ = unsafe { Box::from_raw(cp437_grid.as_ptr()) }; + unsafe { heap_drop(cp437_grid) } } /// Gets the current value at the specified position. diff --git a/src/lib.rs b/src/lib.rs index 4d4e386..f90e93d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -34,6 +34,7 @@ pub use crate::command::*; pub use crate::connection::*; pub use crate::cp437_grid::*; pub use crate::packet::*; +use std::ptr::NonNull; mod bitmap; mod bitvec; @@ -49,3 +50,15 @@ use std::time::Duration; /// Actual hardware limit is around 28-29ms/frame. Rounded up for less dropped packets. pub const SP_FRAME_PACING_MS: u128 = Duration::from_millis(30).as_millis(); + +pub(crate) fn heap_move(x: T) -> *mut T { + Box::into_raw(Box::new(x)) +} + +pub(crate) fn heap_move_nonnull(x: T) -> NonNull { + NonNull::from(Box::leak(Box::new(x))) +} + +pub(crate) unsafe fn heap_drop(x: NonNull) { + drop(unsafe { Box::from_raw(x.as_ptr()) }); +} diff --git a/src/packet.rs b/src/packet.rs index 996c83d..a9def67 100644 --- a/src/packet.rs +++ b/src/packet.rs @@ -1,4 +1,4 @@ -use crate::ByteSlice; +use crate::{heap_drop, heap_move, heap_move_nonnull, ByteSlice}; use servicepoint::{Header, Packet, TypedCommand}; use std::ptr::NonNull; @@ -12,7 +12,7 @@ pub unsafe extern "C" fn sp_packet_from_command( ) -> *mut Packet { let command = unsafe { *Box::from_raw(command.as_ptr()) }; if let Ok(packet) = command.try_into() { - Box::leak(Box::new(packet)) + heap_move(packet) } else { std::ptr::null_mut() } @@ -26,7 +26,7 @@ pub unsafe extern "C" fn sp_packet_try_load(data: ByteSlice) -> *mut Packet { let data = unsafe { data.as_slice() }; match servicepoint::Packet::try_from(data) { Err(_) => std::ptr::null_mut(), - Ok(packet) => Box::into_raw(Box::new(packet)), + Ok(packet) => heap_move(packet), } } @@ -45,8 +45,7 @@ pub unsafe extern "C" fn sp_packet_from_parts( Vec::from(payload) }; - let packet = Box::new(Packet { header, payload }); - NonNull::from(Box::leak(packet)) + heap_move_nonnull(Packet { header, payload }) } /// Returns a pointer to the header field of the provided packet. @@ -100,12 +99,11 @@ pub unsafe extern "C" fn sp_packet_serialize_to( pub unsafe extern "C" fn sp_packet_clone( packet: NonNull, ) -> NonNull { - let result = Box::new(unsafe { packet.as_ref().clone() }); - NonNull::from(Box::leak(result)) + heap_move_nonnull(unsafe { packet.as_ref().clone() }) } /// Deallocates a [Packet]. #[no_mangle] pub unsafe extern "C" fn sp_packet_free(packet: NonNull) { - _ = unsafe { Box::from_raw(packet.as_ptr()) } + unsafe { heap_drop(packet) } }