Wrap slice::from_raw_parts to be compatible with rcl (#419)

* Wrap slice::from_raw_parts to be compatible with rcl

Signed-off-by: Michael X. Grey <[email protected]>

* Fix style

Signed-off-by: Michael X. Grey <[email protected]>

---------

Signed-off-by: Michael X. Grey <[email protected]>

Author: mxgrey

rclrs/src/node/graph.rs

@@ -1,7 +1,6 @@
 use std::{
     collections::HashMap,
     ffi::{CStr, CString},
-    slice,
 };
 
 use crate::{rcl_bindings::*, Node, RclrsError, ToResult};
@@ -182,8 +181,8 @@ impl Node {
         // namespaces are populated with valid data
         let (names_slice, namespaces_slice) = unsafe {
             (
-                slice::from_raw_parts(rcl_names.data, rcl_names.size),
-                slice::from_raw_parts(rcl_namespaces.data, rcl_namespaces.size),
+                rcl_from_raw_parts(rcl_names.data, rcl_names.size),
+                rcl_from_raw_parts(rcl_namespaces.data, rcl_namespaces.size),
             )
         };
 
@@ -230,9 +229,9 @@ impl Node {
         // SAFETY: The previous function successfully returned, so the arrays are valid
         let (names_slice, namespaces_slice, enclaves_slice) = unsafe {
             (
-                slice::from_raw_parts(rcl_names.data, rcl_names.size),
-                slice::from_raw_parts(rcl_namespaces.data, rcl_namespaces.size),
-                slice::from_raw_parts(rcl_enclaves.data, rcl_enclaves.size),
+                rcl_from_raw_parts(rcl_names.data, rcl_names.size),
+                rcl_from_raw_parts(rcl_namespaces.data, rcl_namespaces.size),
+                rcl_from_raw_parts(rcl_enclaves.data, rcl_enclaves.size),
             )
         };
 
@@ -379,10 +378,9 @@ impl Node {
             .ok()?;
         }
 
-        // SAFETY: The previous call returned successfully, so the slice is valid
-        let topic_endpoint_infos_slice = unsafe {
-            slice::from_raw_parts(rcl_publishers_info.info_array, rcl_publishers_info.size)
-        };
+        // SAFETY: The previous call returned successfully, so the data is valid
+        let topic_endpoint_infos_slice =
+            unsafe { rcl_from_raw_parts(rcl_publishers_info.info_array, rcl_publishers_info.size) };
 
         // SAFETY: Because the rcl call returned successfully, each element of the slice points
         // to a valid topic_endpoint_info object, which contains valid C strings
@@ -422,7 +420,7 @@ fn convert_names_and_types(
 
     // SAFETY: Safe if the rcl_names_and_types arg has been initialized by the caller
     let name_slice = unsafe {
-        slice::from_raw_parts(
+        rcl_from_raw_parts(
             rcl_names_and_types.names.data,
             rcl_names_and_types.names.size,
         )
@@ -438,7 +436,7 @@ fn convert_names_and_types(
         // SAFETY: Safe as long as rcl_names_and_types was populated by the caller
         let types: Vec<String> = unsafe {
             let p = rcl_names_and_types.types.add(idx);
-            slice::from_raw_parts((*p).data, (*p).size)
+            rcl_from_raw_parts((*p).data, (*p).size)
                 .iter()
                 .map(|s| {
                     let cstr = CStr::from_ptr(*s);

rclrs/src/parameter/override_map.rs

@@ -49,9 +49,8 @@ impl RclParamsIter<'_> {
             }
         } else {
             let node_name_ptrs =
-                std::slice::from_raw_parts((*rcl_params).node_names, (*rcl_params).num_nodes);
-            let rcl_node_params =
-                std::slice::from_raw_parts((*rcl_params).params, (*rcl_params).num_nodes);
+                rcl_from_raw_parts((*rcl_params).node_names, (*rcl_params).num_nodes);
+            let rcl_node_params = rcl_from_raw_parts((*rcl_params).params, (*rcl_params).num_nodes);
             Self {
                 node_name_ptrs,
                 rcl_node_params,
@@ -83,11 +82,9 @@ impl<'a> RclNodeParamsIter<'a> {
     // sizes or dangling pointers.
     pub unsafe fn new(rcl_node_params: &'a rcl_node_params_t) -> Self {
         let param_name_ptrs =
-            std::slice::from_raw_parts(rcl_node_params.parameter_names, rcl_node_params.num_params);
-        let rcl_variants = std::slice::from_raw_parts(
-            rcl_node_params.parameter_values,
-            rcl_node_params.num_params,
-        );
+            rcl_from_raw_parts(rcl_node_params.parameter_names, rcl_node_params.num_params);
+        let rcl_variants =
+            rcl_from_raw_parts(rcl_node_params.parameter_values, rcl_node_params.num_params);
         Self {
             param_name_ptrs,
             rcl_variants,

rclrs/src/parameter/value.rs

@@ -460,25 +460,23 @@ impl ParameterValue {
             ParameterValue::String(s.into())
         } else if !var.byte_array_value.is_null() {
             let rcl_byte_array = &*var.byte_array_value;
-            let slice = std::slice::from_raw_parts(rcl_byte_array.values, rcl_byte_array.size);
+            let slice = rcl_from_raw_parts(rcl_byte_array.values, rcl_byte_array.size);
             ParameterValue::ByteArray(slice.into())
         } else if !var.bool_array_value.is_null() {
             let rcl_bool_array = &*var.bool_array_value;
-            let slice = std::slice::from_raw_parts(rcl_bool_array.values, rcl_bool_array.size);
+            let slice = rcl_from_raw_parts(rcl_bool_array.values, rcl_bool_array.size);
             ParameterValue::BoolArray(slice.into())
         } else if !var.integer_array_value.is_null() {
             let rcl_integer_array = &*var.integer_array_value;
-            let slice =
-                std::slice::from_raw_parts(rcl_integer_array.values, rcl_integer_array.size);
+            let slice = rcl_from_raw_parts(rcl_integer_array.values, rcl_integer_array.size);
             ParameterValue::IntegerArray(slice.into())
         } else if !var.double_array_value.is_null() {
             let rcl_double_array = &*var.double_array_value;
-            let slice = std::slice::from_raw_parts(rcl_double_array.values, rcl_double_array.size);
+            let slice = rcl_from_raw_parts(rcl_double_array.values, rcl_double_array.size);
             ParameterValue::DoubleArray(slice.into())
         } else if !var.string_array_value.is_null() {
             let rcutils_string_array = &*var.string_array_value;
-            let slice =
-                std::slice::from_raw_parts(rcutils_string_array.data, rcutils_string_array.size);
+            let slice = rcl_from_raw_parts(rcutils_string_array.data, rcutils_string_array.size);
             let strings = slice
                 .iter()
                 .map(|&ptr| {

rclrs/src/rcl_bindings.rs

@@ -148,3 +148,23 @@ cfg_if::cfg_if! {
         pub const RMW_GID_STORAGE_SIZE: usize = rmw_gid_storage_size_constant;
     }
 }
+
+/// Wrapper around [`std::slice::from_raw_parts`] that accommodates the rcl
+/// convention of providing a null pointer to represent empty arrays. This
+/// violates the safety requirements of [`std::slice::from_raw_parts`].
+///
+/// # Safety
+///
+/// Behavior is undefined in all the same scenarios as [`slice::from_raw_parts`]
+/// (see its safety section) except that null pointers are allowed and will
+/// return a slice to an empty array.
+pub(crate) unsafe fn rcl_from_raw_parts<'a, T>(data: *const T, len: usize) -> &'a [T] {
+    if data.is_null() {
+        &[]
+    } else {
+        // SAFETY: The user of this function is instructed to abide by all the
+        // safety requirements of slice::from_raw_parts except for null pointer
+        // values, which are checked above.
+        unsafe { std::slice::from_raw_parts(data, len) }
+    }
+}