Skip to content

Commit

Permalink
Wrap slice::from_raw_parts to be compatible with rcl (#419)
Browse files Browse the repository at this point in the history
* 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]>
  • Loading branch information
mxgrey authored Oct 3, 2024
1 parent 5903d73 commit ad9667f
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 27 deletions.
22 changes: 10 additions & 12 deletions rclrs/src/node/graph.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::{
collections::HashMap,
ffi::{CStr, CString},
slice,
};

use crate::{rcl_bindings::*, Node, RclrsError, ToResult};
Expand Down Expand Up @@ -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),
)
};

Expand Down Expand Up @@ -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),
)
};

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
)
Expand All @@ -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);
Expand Down
13 changes: 5 additions & 8 deletions rclrs/src/parameter/override_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 5 additions & 7 deletions rclrs/src/parameter/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
20 changes: 20 additions & 0 deletions rclrs/src/rcl_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
}
}

0 comments on commit ad9667f

Please sign in to comment.