Merge pull request #87 from Ragarnoy/86-unsafe-code-rework

Add debug assertions for null pointers, enhance doc

src/config.rs

@@ -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) };
     }
 }
@@ -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
     }
 

src/detector/distance.rs

@@ -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) }
     }
 }

src/detector/distance/config.rs

@@ -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) }
     }
 }

src/detector/presence.rs

@@ -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) }
     }
 }

src/detector/presence/config.rs

@@ -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) }
     }
 }

src/num.rs

@@ -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
     }

src/processing.rs

@@ -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);
         }

src/processing/metadata.rs

@@ -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
     }

src/radar.rs

@@ -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() {
@@ -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()
     }
 }

src/sensor.rs

@@ -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);
         }
@@ -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 }
     }
@@ -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()) };
@@ -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 {
@@ -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
     }
 }