From 5cddd54a8df8da0f4623d5729442f982950c3195 Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Thu, 3 Oct 2024 15:42:10 +0800 Subject: [PATCH] Wrap slice::from_raw_parts to be compatible with rcl Signed-off-by: Michael X. Grey --- rclrs/src/node/graph.rs | 19 +++++++++---------- rclrs/src/parameter/override_map.rs | 8 ++++---- rclrs/src/parameter/value.rs | 10 +++++----- rclrs/src/rcl_bindings.rs | 22 ++++++++++++++++++++++ 4 files changed, 40 insertions(+), 19 deletions(-) diff --git a/rclrs/src/node/graph.rs b/rclrs/src/node/graph.rs index c8079b32a..6fb2cc0a4 100644 --- a/rclrs/src/node/graph.rs +++ b/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,9 +378,9 @@ impl Node { .ok()?; } - // SAFETY: The previous call returned successfully, so the slice is valid + // SAFETY: The previous call returned successfully, so the data is valid let topic_endpoint_infos_slice = unsafe { - slice::from_raw_parts(rcl_publishers_info.info_array, rcl_publishers_info.size) + 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 @@ -422,7 +421,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 +437,7 @@ fn convert_names_and_types( // SAFETY: Safe as long as rcl_names_and_types was populated by the caller let types: Vec = 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); diff --git a/rclrs/src/parameter/override_map.rs b/rclrs/src/parameter/override_map.rs index 64c3cfdfe..aa577c97a 100644 --- a/rclrs/src/parameter/override_map.rs +++ b/rclrs/src/parameter/override_map.rs @@ -49,9 +49,9 @@ impl RclParamsIter<'_> { } } else { let node_name_ptrs = - std::slice::from_raw_parts((*rcl_params).node_names, (*rcl_params).num_nodes); + rcl_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).params, (*rcl_params).num_nodes); Self { node_name_ptrs, rcl_node_params, @@ -83,8 +83,8 @@ 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_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, ); diff --git a/rclrs/src/parameter/value.rs b/rclrs/src/parameter/value.rs index 9d431c03e..1e1c95f95 100644 --- a/rclrs/src/parameter/value.rs +++ b/rclrs/src/parameter/value.rs @@ -460,25 +460,25 @@ 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); + 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); + rcl_from_raw_parts(rcutils_string_array.data, rcutils_string_array.size); let strings = slice .iter() .map(|&ptr| { diff --git a/rclrs/src/rcl_bindings.rs b/rclrs/src/rcl_bindings.rs index 94491bc91..59930638d 100644 --- a/rclrs/src/rcl_bindings.rs +++ b/rclrs/src/rcl_bindings.rs @@ -148,3 +148,25 @@ 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) + } + } +}