Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add debug assertions for null pointers, enhance doc #87

Merged
merged 1 commit into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ impl Default for RadarConfig {
impl Drop for RadarConfig {
/// Destroys the radar configuration instance, freeing any allocated resources.
fn drop(&mut self) {
debug_assert!(!self.inner.is_null(), "Radar configuration is null");
unsafe { acc_config_destroy(self.inner) };
}
}
Expand All @@ -95,6 +96,7 @@ impl RadarConfig {
/// # Safety
/// This function is unsafe because it returns a raw pointer.
pub unsafe fn mut_ptr(&mut self) -> *mut acc_config_t {
debug_assert!(!self.inner.is_null(), "Radar configuration is null");
self.inner
}

Expand Down
1 change: 1 addition & 0 deletions src/detector/distance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ impl InnerRadarDistanceDetector {

impl Drop for InnerRadarDistanceDetector {
fn drop(&mut self) {
debug_assert!(!self.inner.is_null(), "Inner detector handle is null");
unsafe { acc_detector_distance_destroy(self.inner) }
}
}
Expand Down
1 change: 1 addition & 0 deletions src/detector/distance/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ pub struct RadarDistanceConfig {

impl Drop for RadarDistanceConfig {
fn drop(&mut self) {
debug_assert!(self.inner.is_null(), "Dropping a non-null pointer");
unsafe { acc_detector_distance_config_destroy(self.inner) }
}
}
Expand Down
1 change: 1 addition & 0 deletions src/detector/presence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ impl InnerPresenceDetector {

impl Drop for InnerPresenceDetector {
fn drop(&mut self) {
debug_assert!(!self.inner.is_null(), "Detector handle is null");
unsafe { acc_detector_presence_destroy(self.inner) }
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/detector/presence/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ pub struct PresenceConfig {

impl Drop for PresenceConfig {
fn drop(&mut self) {
debug_assert!(
!self.inner.is_null(),
"Presence detector configuration is null"
);
unsafe { acc_detector_presence_config_destroy(self.inner) }
}
}
Expand Down
15 changes: 10 additions & 5 deletions src/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,23 @@ impl AccComplex {
Self::default()
}

/// Creates a new `AccComplex` instance from a raw pointer.
/// Creates a new AccComplex from a raw pointer.
///
/// # Safety
/// This function is unsafe because it dereferences a raw pointer.
/// The caller must ensure that the pointer is valid and points to a properly initialized `acc_int16_complex_t` struct.
/// - ptr must be a valid pointer to an initialized acc_int16_complex_t
/// - The pointed-to data must be valid for the lifetime of the returned AccComplex
/// - The pointer must be properly aligned
pub unsafe fn from_ptr(ptr: *const acc_int16_complex_t) -> Self {
debug_assert!(!ptr.is_null(), "Pointer is null");
Self { inner: *ptr }
}

/// Returns a mutable pointer to the inner `acc_int16_complex_t` struct.
/// Returns a mutable pointer to the inner complex number data.
///
/// # Safety
/// This function is unsafe because it returns a raw pointer.
/// - The pointer is only valid for the lifetime of this AccComplex instance
/// - The caller must ensure no other references to the inner data exist
/// - The data must not be modified in a way that violates AccComplex invariants
pub unsafe fn mut_ptr(&mut self) -> *mut acc_int16_complex_t {
&mut self.inner
}
Expand Down
1 change: 1 addition & 0 deletions src/processing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ impl Processing {

impl Drop for Processing {
fn drop(&mut self) {
debug_assert!(!self.inner.is_null(), "Processing is null");
unsafe {
acc_processing_destroy(self.inner);
}
Expand Down
7 changes: 5 additions & 2 deletions src/processing/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@ impl ProcessingMetaData {
Self::default()
}

/// Get a mutable reference to the metadata
/// Gets a mutable pointer to the underlying metadata structure.
///
/// # Safety
/// This function is unsafe because it returns a mutable reference to the metadata, which is a raw pointer
/// - The caller must ensure the pointer is only used while ProcessingMetaData exists
/// - The pointer must only be used for passing to Acconeer API functions
/// - No other references to the data can exist while the pointer is in use
pub unsafe fn mut_ptr(&mut self) -> *mut acc_processing_metadata_t {
&mut self.inner
}
Expand Down
28 changes: 26 additions & 2 deletions src/radar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,25 @@ where
ENABLE: OutputPin,
DLY: DelayNs,
{
/// Starts a radar measurement with a previously prepared configuration.
///
/// This function initiates a radar measurement based on a configuration that must have been
/// set up and prepared in advance. Ensure the sensor is powered on and calibration and
/// preparation steps have been completed before calling this function.
///
/// # Preconditions
///
/// - The sensor must be powered on.
/// - `calibrate` must have been successfully called.
/// - `prepare` must have been successfully called.
///
/// # Arguments
///
/// * `sensor` - The sensor instance to use for measurement.
///
/// # Returns
///
/// `Ok(())` if the measurement was successfully started, `Err(SensorError)` otherwise.
pub async fn measure<'a>(&mut self, data: &mut [u8]) -> Result<(), SensorError> {
if (self.sensor.measure(&mut self.interrupt).await).is_ok() {
if self.sensor.read(data).is_ok() {
Expand Down Expand Up @@ -284,10 +303,15 @@ where
self.sensor.check_status();
}

/// Get a mutable reference to the sensor
/// Gets a mutable pointer to the underlying sensor.
///
/// # Safety
/// This function is unsafe because it returns a mutable reference to the sensor, which is a raw pointer
/// - The returned pointer is only valid while the Radar instance exists
/// - The pointer must not be used after the sensor is disabled or reset
/// - Only one mutable reference can exist at a time
/// - The caller must not violate the state transition requirements
pub unsafe fn inner_sensor(&self) -> *mut acc_sensor_t {
debug_assert!(!self.sensor.inner().is_null(), "Sensor pointer is null");
self.sensor.inner()
}
}
Expand Down
87 changes: 38 additions & 49 deletions src/sensor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,35 @@ use a121_sys::*;
pub mod calibration;
pub mod error;

struct InnerSensor {
/// Safety-critical wrapper around raw sensor pointer
pub struct InnerSensor {
/// Raw pointer to acc_sensor_t. Must never be null once initialized.
/// Managed exclusively by this type to maintain memory safety.
inner: *mut acc_sensor_t,
}

impl InnerSensor {
/// Creates a new sensor instance from a sensor ID.
///
/// # Safety
/// - The returned pointer must be valid and non-null
/// - The sensor must be powered on before calling this function
/// - The sensor must not be used in another sensor instance without a power/reset cycle
pub fn new(sensor_id: u32) -> Option<Self> {
let sensor_ptr = unsafe { acc_sensor_create(sensor_id as acc_sensor_id_t) };

// Runtime safety check for null pointer
if sensor_ptr.is_null() {
None
} else {
Some(Self { inner: sensor_ptr })
return None;
}

Some(Self { inner: sensor_ptr })
}
}

impl Drop for InnerSensor {
fn drop(&mut self) {
debug_assert!(!self.inner.is_null(), "Sensor pointer is null in drop");
unsafe {
acc_sensor_destroy(self.inner);
}
Expand All @@ -41,12 +53,22 @@ impl Drop for InnerSensor {
impl Deref for InnerSensor {
type Target = acc_sensor_t;

/// # Safety
/// Dereference is safe because:
/// - inner is guaranteed non-null by constructor
/// - exclusive access is maintained by Rust ownership rules
/// - pointer remains valid until drop
fn deref(&self) -> &Self::Target {
unsafe { &*self.inner }
}
}

impl DerefMut for InnerSensor {
/// # Safety
/// Dereference is safe because:
/// - inner is guaranteed non-null by constructor
/// - exclusive access is maintained by Rust ownership rules
/// - pointer remains valid until drop
fn deref_mut(&mut self) -> &mut Self::Target {
unsafe { &mut *self.inner }
}
Expand Down Expand Up @@ -246,37 +268,6 @@ where
}
}

/// Starts a radar measurement with a previously prepared configuration.
///
/// This function initiates a radar measurement based on a configuration that must have been
/// set up and prepared in advance. Ensure the sensor is powered on and calibration and
/// preparation steps have been completed before calling this function.
///
/// # Preconditions
///
/// - The sensor must be powered on.
/// - `calibrate` must have been successfully called.
/// - `prepare` must have been successfully called.
///
/// # Arguments
///
/// * `sensor` - The sensor instance to use for measurement.
///
/// # Returns
///
/// `Ok(())` if the measurement was successfully started, `Err(SensorError)` otherwise.
///
/// # Example
///
/// ```no_run
/// # use embedded_hal_async::digital::Wait;
/// use rad_hard_sys::sensor::*;
/// use rad_hard_sys::sensor::error::SensorError;
/// async fn foo<SINT: Wait>(sensor: &mut Sensor<Ready, SINT>) -> Result<(), SensorError> {
/// sensor.measure().await?;
/// # Ok(())
/// # }
/// ```
pub async fn measure<SINT: Wait>(&mut self, mut interrupt: SINT) -> Result<(), SensorError> {
// Implementation to start the radar measurement
let success = unsafe { acc_sensor_measure(self.inner.deref_mut()) };
Expand Down Expand Up @@ -307,20 +298,6 @@ where
///
/// # Returns
/// `Ok(())` if data was successfully read into the buffer, `Err(SensorError)` otherwise.
///
/// # Example
///
/// ```no_run
/// # use embedded_hal_async::digital::Wait;
/// use rad_hard_sys::sensor::*;
/// use rad_hard_sys::sensor::data::RadarData;
/// use rad_hard_sys::sensor::error::SensorError;
/// async fn foo<SINT: Wait>(sensor: &mut Sensor<Ready, SINT>) -> Result<(), SensorError> {
/// let mut data_buffer = RadarData::default();
/// sensor.read(&mut data_buffer)?;
/// # Ok(())
/// # }
/// ```
pub fn read(&self, buffer: &mut [u8]) -> Result<(), SensorError> {
// Implementation to read the radar data
let success = unsafe {
Expand All @@ -337,7 +314,19 @@ where
}
}

/// Returns a mutable pointer to the underlying sensor.
///
/// # Safety
/// This function is unsafe because:
/// - The caller must ensure the sensor has been properly initialized
/// - The pointer remains valid only for the lifetime of this Sensor instance
/// - The caller must not use this pointer after the Sensor is dropped
/// - Multiple mutable references to the same sensor must not be created
pub unsafe fn inner(&self) -> *mut acc_sensor_t {
debug_assert!(
!self.inner.inner.is_null(),
"Sensor has not been initialized"
);
self.inner.inner
}
}
Loading