From e6f6553e842fbe3ed1ef549c6802e15b5a606922 Mon Sep 17 00:00:00 2001 From: Tatsuro Sakaguchi Date: Sun, 16 Oct 2022 02:41:05 +0000 Subject: [PATCH 1/9] Replace libc with std::ffi --- rclrs/src/arguments.rs | 3 +-- rclrs/src/node.rs | 2 +- rclrs/src/parameter/override_map.rs | 3 +-- rosidl_runtime_rs/src/string.rs | 10 +++++----- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/rclrs/src/arguments.rs b/rclrs/src/arguments.rs index 81c23420a..cb91cdc2e 100644 --- a/rclrs/src/arguments.rs +++ b/rclrs/src/arguments.rs @@ -1,9 +1,8 @@ +use std::ffi::c_void; use std::ffi::CString; use std::os::raw::c_char; use std::ptr::null_mut; -use libc::c_void; - use crate::error::*; use crate::rcl_bindings::*; diff --git a/rclrs/src/node.rs b/rclrs/src/node.rs index 323c3623b..8d85d85f9 100644 --- a/rclrs/src/node.rs +++ b/rclrs/src/node.rs @@ -1,12 +1,12 @@ mod builder; mod graph; use std::cmp::PartialEq; +use std::ffi::c_char; use std::ffi::CStr; use std::fmt; use std::sync::{Arc, Mutex, Weak}; use std::vec::Vec; -use libc::c_char; use rosidl_runtime_rs::Message; pub use self::builder::*; diff --git a/rclrs/src/parameter/override_map.rs b/rclrs/src/parameter/override_map.rs index 3311e6c76..868008813 100644 --- a/rclrs/src/parameter/override_map.rs +++ b/rclrs/src/parameter/override_map.rs @@ -1,8 +1,7 @@ use std::collections::BTreeMap; +use std::ffi::c_char; use std::ffi::CStr; -use libc::c_char; - use crate::rcl_bindings::*; use crate::{ParameterValue, RclrsError, ToResult}; diff --git a/rosidl_runtime_rs/src/string.rs b/rosidl_runtime_rs/src/string.rs index 305062a14..76cc90ea2 100644 --- a/rosidl_runtime_rs/src/string.rs +++ b/rosidl_runtime_rs/src/string.rs @@ -29,7 +29,7 @@ use crate::traits::SequenceAlloc; pub struct String { /// Dynamic memory in this type is allocated and deallocated by C, but this is a detail that is managed by /// the relevant functions and trait impls. - data: *mut libc::c_char, + data: *mut std::ffi::c_char, size: libc::size_t, capacity: libc::size_t, } @@ -50,7 +50,7 @@ pub struct String { /// ``` #[repr(C)] pub struct WString { - data: *mut libc::c_ushort, + data: *mut std::ffi::c_ushort, size: libc::size_t, capacity: libc::size_t, } @@ -245,7 +245,7 @@ macro_rules! string_impl { string_impl!( String, - libc::c_char, + std::ffi::c_char, u8, from_utf8_lossy, rosidl_runtime_c__String__init, @@ -257,7 +257,7 @@ string_impl!( ); string_impl!( WString, - libc::c_ushort, + std::ffi::c_ushort, u16, from_utf16_lossy, rosidl_runtime_c__U16String__init, @@ -330,7 +330,7 @@ impl Debug for BoundedString { } impl Deref for BoundedString { - type Target = [libc::c_char]; + type Target = [std::ffi::c_char]; fn deref(&self) -> &Self::Target { self.inner.deref() } From 1957a8210319b2d739b4d9d2957b4fbae36419f4 Mon Sep 17 00:00:00 2001 From: Tatsuro Sakaguchi Date: Sun, 16 Oct 2022 02:45:15 +0000 Subject: [PATCH 2/9] Replace existing std::os::raw with std::ffi --- rclrs/src/arguments.rs | 6 +++--- rclrs/src/context.rs | 2 +- rclrs/src/node/graph.rs | 14 +++++++------- rclrs/src/subscription/message_info.rs | 2 +- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/rclrs/src/arguments.rs b/rclrs/src/arguments.rs index cb91cdc2e..1d8b8e301 100644 --- a/rclrs/src/arguments.rs +++ b/rclrs/src/arguments.rs @@ -1,6 +1,6 @@ +use std::ffi::c_char; use std::ffi::c_void; use std::ffi::CString; -use std::os::raw::c_char; use std::ptr::null_mut; use crate::error::*; @@ -88,11 +88,11 @@ pub fn extract_non_ros_args( /// `rcl_arguments_get_count_unparsed_ros` -> `rcl_arguments_get_count_ros` /// ... pub(crate) fn get_rcl_arguments( - rcl_get_count: unsafe extern "C" fn(*const rcl_arguments_t) -> std::os::raw::c_int, + rcl_get_count: unsafe extern "C" fn(*const rcl_arguments_t) -> std::ffi::c_int, rcl_get_indices: unsafe extern "C" fn( *const rcl_arguments_t, rcl_allocator_t, - *mut *mut std::os::raw::c_int, + *mut *mut std::ffi::c_int, ) -> rcl_ret_t, rcl_arguments: *const rcl_arguments_t, args: &[String], diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index 83ef17c56..88104c399 100644 --- a/rclrs/src/context.rs +++ b/rclrs/src/context.rs @@ -1,5 +1,5 @@ +use std::ffi::c_char; use std::ffi::CString; -use std::os::raw::c_char; use std::string::String; use std::sync::{Arc, Mutex}; use std::vec::Vec; diff --git a/rclrs/src/node/graph.rs b/rclrs/src/node/graph.rs index cacf96837..fdfe3c7ac 100644 --- a/rclrs/src/node/graph.rs +++ b/rclrs/src/node/graph.rs @@ -68,8 +68,8 @@ impl Node { unsafe extern "C" fn wrapper( node: *const rcl_node_t, allocator: *mut rcl_allocator_t, - node_name: *const ::std::os::raw::c_char, - node_namespace: *const ::std::os::raw::c_char, + node_name: *const ::std::ffi::c_char, + node_namespace: *const ::std::ffi::c_char, topic_names_and_types: *mut rcl_names_and_types_t, ) -> rcl_ret_t { rcl_get_publisher_names_and_types_by_node( @@ -95,8 +95,8 @@ impl Node { unsafe extern "C" fn wrapper( node: *const rcl_node_t, allocator: *mut rcl_allocator_t, - node_name: *const ::std::os::raw::c_char, - node_namespace: *const ::std::os::raw::c_char, + node_name: *const ::std::ffi::c_char, + node_namespace: *const ::std::ffi::c_char, topic_names_and_types: *mut rcl_names_and_types_t, ) -> rcl_ret_t { rcl_get_subscriber_names_and_types_by_node( @@ -316,8 +316,8 @@ impl Node { getter: unsafe extern "C" fn( *const rcl_node_t, *mut rcl_allocator_t, - *const ::std::os::raw::c_char, - *const ::std::os::raw::c_char, + *const ::std::ffi::c_char, + *const ::std::ffi::c_char, *mut rcl_names_and_types_t, ) -> rcl_ret_t, ) -> Result { @@ -355,7 +355,7 @@ impl Node { getter: unsafe extern "C" fn( *const rcl_node_t, *mut rcl_allocator_t, - *const ::std::os::raw::c_char, + *const ::std::ffi::c_char, bool, *mut rcl_topic_endpoint_info_array_t, ) -> rcl_ret_t, diff --git a/rclrs/src/subscription/message_info.rs b/rclrs/src/subscription/message_info.rs index b1dae04cb..b2e08cf47 100644 --- a/rclrs/src/subscription/message_info.rs +++ b/rclrs/src/subscription/message_info.rs @@ -37,7 +37,7 @@ pub struct PublisherGid { /// do not need it. /// /// [1]: std::ffi::CString - pub implementation_identifier: *const std::os::raw::c_char, + pub implementation_identifier: *const std::ffi::c_char, } // SAFETY: The implementation identifier doesn't care about which thread it's read from. From 79579ab15786d56472ac3c0029370e77a53be673 Mon Sep 17 00:00:00 2001 From: Tatsuro Sakaguchi Date: Sun, 16 Oct 2022 02:49:06 +0000 Subject: [PATCH 3/9] Replace uintptr_t and size_t with usize --- rosidl_generator_rs/resource/msg_rmw.rs.em | 8 ++++---- rosidl_generator_rs/resource/srv.rs.em | 8 ++++---- rosidl_runtime_rs/src/sequence.rs | 8 ++++---- rosidl_runtime_rs/src/string.rs | 18 +++++++++--------- rosidl_runtime_rs/src/traits.rs | 6 +++--- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/rosidl_generator_rs/resource/msg_rmw.rs.em b/rosidl_generator_rs/resource/msg_rmw.rs.em index 24d416d01..3339bd4c2 100644 --- a/rosidl_generator_rs/resource/msg_rmw.rs.em +++ b/rosidl_generator_rs/resource/msg_rmw.rs.em @@ -19,13 +19,13 @@ type_name = msg_spec.structure.namespaced_type.name #[link(name = "@(package_name)__rosidl_typesupport_c")] extern "C" { - fn rosidl_typesupport_c__get_message_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> libc::uintptr_t; + fn rosidl_typesupport_c__get_message_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> usize; } #[link(name = "@(package_name)__rosidl_generator_c")] extern "C" { fn @(package_name)__@(subfolder)__@(type_name)__init(msg: *mut @(type_name)) -> bool; - fn @(package_name)__@(subfolder)__@(type_name)__Sequence__init(seq: *mut rosidl_runtime_rs::Sequence<@(type_name)>, size: libc::size_t) -> bool; + fn @(package_name)__@(subfolder)__@(type_name)__Sequence__init(seq: *mut rosidl_runtime_rs::Sequence<@(type_name)>, size: usize) -> bool; fn @(package_name)__@(subfolder)__@(type_name)__Sequence__fini(seq: *mut rosidl_runtime_rs::Sequence<@(type_name)>); } @@ -80,7 +80,7 @@ impl Default for @(type_name) { } impl rosidl_runtime_rs::SequenceAlloc for @(type_name) { - fn sequence_init(seq: &mut rosidl_runtime_rs::Sequence, size: libc::size_t) -> bool { + fn sequence_init(seq: &mut rosidl_runtime_rs::Sequence, size: usize) -> bool { // SAFETY: This is safe since a the point is guaranteed to be valid/initialized. unsafe { @(package_name)__@(subfolder)__@(type_name)__Sequence__init(seq as *mut _, size) } } @@ -103,7 +103,7 @@ impl rosidl_runtime_rs::Message for @(type_name) { impl rosidl_runtime_rs::RmwMessage for @(type_name) where Self: Sized { const TYPE_NAME: &'static str = "@(package_name)/@(subfolder)/@(type_name)"; - fn get_type_support() -> libc::uintptr_t { + fn get_type_support() -> usize { // SAFETY: No preconditions for this function. unsafe { rosidl_typesupport_c__get_message_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() } } diff --git a/rosidl_generator_rs/resource/srv.rs.em b/rosidl_generator_rs/resource/srv.rs.em index 925b8cdd3..00d0a409f 100644 --- a/rosidl_generator_rs/resource/srv.rs.em +++ b/rosidl_generator_rs/resource/srv.rs.em @@ -24,7 +24,7 @@ type_name = srv_spec.namespaced_type.name #[link(name = "@(package_name)__rosidl_typesupport_c")] extern "C" { - fn rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> libc::uintptr_t; + fn rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> usize; } // Corresponds to @(package_name)__@(subfolder)__@(type_name) @@ -34,7 +34,7 @@ impl rosidl_runtime_rs::Service for @(type_name) { type Request = crate::@(subfolder)::@(type_name)_Request; type Response = crate::@(subfolder)::@(type_name)_Response; - fn get_type_support() -> libc::uintptr_t { + fn get_type_support() -> usize { // SAFETY: No preconditions for this function. unsafe { rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() } } @@ -61,7 +61,7 @@ type_name = srv_spec.namespaced_type.name #[link(name = "@(package_name)__rosidl_typesupport_c")] extern "C" { - fn rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> libc::uintptr_t; + fn rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> usize; } // Corresponds to @(package_name)__@(subfolder)__@(type_name) @@ -71,7 +71,7 @@ type_name = srv_spec.namespaced_type.name type Request = crate::@(subfolder)::rmw::@(type_name)_Request; type Response = crate::@(subfolder)::rmw::@(type_name)_Response; - fn get_type_support() -> libc::uintptr_t { + fn get_type_support() -> usize { // SAFETY: No preconditions for this function. unsafe { rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() } } diff --git a/rosidl_runtime_rs/src/sequence.rs b/rosidl_runtime_rs/src/sequence.rs index 64b5892b9..db38704f0 100644 --- a/rosidl_runtime_rs/src/sequence.rs +++ b/rosidl_runtime_rs/src/sequence.rs @@ -37,8 +37,8 @@ use crate::traits::SequenceAlloc; #[repr(C)] pub struct Sequence { data: *mut T, - size: libc::size_t, - capacity: libc::size_t, + size: usize, + capacity: usize, } /// A bounded sequence. @@ -518,12 +518,12 @@ macro_rules! impl_sequence_alloc_for_primitive_type { ($rust_type:ty, $init_func:ident, $fini_func:ident, $copy_func:ident) => { #[link(name = "rosidl_runtime_c")] extern "C" { - fn $init_func(seq: *mut Sequence<$rust_type>, size: libc::size_t) -> bool; + fn $init_func(seq: *mut Sequence<$rust_type>, size: usize) -> bool; fn $fini_func(seq: *mut Sequence<$rust_type>); } impl SequenceAlloc for $rust_type { - fn sequence_init(seq: &mut Sequence, size: libc::size_t) -> bool { + fn sequence_init(seq: &mut Sequence, size: usize) -> bool { // SAFETY: There are no special preconditions to the sequence_init function. unsafe { // This allocates space and sets seq.size and seq.capacity to size diff --git a/rosidl_runtime_rs/src/string.rs b/rosidl_runtime_rs/src/string.rs index 76cc90ea2..87b75442e 100644 --- a/rosidl_runtime_rs/src/string.rs +++ b/rosidl_runtime_rs/src/string.rs @@ -30,8 +30,8 @@ pub struct String { /// Dynamic memory in this type is allocated and deallocated by C, but this is a detail that is managed by /// the relevant functions and trait impls. data: *mut std::ffi::c_char, - size: libc::size_t, - capacity: libc::size_t, + size: usize, + capacity: usize, } /// A zero-terminated string of 16-bit characters. @@ -51,8 +51,8 @@ pub struct String { #[repr(C)] pub struct WString { data: *mut std::ffi::c_ushort, - size: libc::size_t, - capacity: libc::size_t, + size: usize, + capacity: usize, } /// A zero-terminated string of 8-bit characters with a length limit. @@ -117,8 +117,8 @@ macro_rules! string_impl { extern "C" { fn $init(s: *mut $string) -> bool; fn $fini(s: *mut $string); - fn $assignn(s: *mut $string, value: *const $char_type, n: libc::size_t) -> bool; - fn $sequence_init(seq: *mut Sequence<$string>, size: libc::size_t) -> bool; + fn $assignn(s: *mut $string, value: *const $char_type, n: usize) -> bool; + fn $sequence_init(seq: *mut Sequence<$string>, size: usize) -> bool; fn $sequence_fini(seq: *mut Sequence<$string>); } @@ -226,7 +226,7 @@ macro_rules! string_impl { unsafe impl Sync for $string {} impl SequenceAlloc for $string { - fn sequence_init(seq: &mut Sequence, size: libc::size_t) -> bool { + fn sequence_init(seq: &mut Sequence, size: usize) -> bool { // SAFETY: There are no special preconditions to the sequence_init function. unsafe { $sequence_init(seq as *mut _, size) } } @@ -349,7 +349,7 @@ impl Display for BoundedString { } impl SequenceAlloc for BoundedString { - fn sequence_init(seq: &mut Sequence, size: libc::size_t) -> bool { + fn sequence_init(seq: &mut Sequence, size: usize) -> bool { // SAFETY: There are no special preconditions to the rosidl_runtime_c__String__Sequence__init function. unsafe { rosidl_runtime_c__String__Sequence__init(seq as *mut Sequence as *mut _, size) @@ -415,7 +415,7 @@ impl Display for BoundedWString { } impl SequenceAlloc for BoundedWString { - fn sequence_init(seq: &mut Sequence, size: libc::size_t) -> bool { + fn sequence_init(seq: &mut Sequence, size: usize) -> bool { // SAFETY: There are no special preconditions to the rosidl_runtime_c__U16String__Sequence__init function. unsafe { rosidl_runtime_c__U16String__Sequence__init(seq as *mut Sequence as *mut _, size) diff --git a/rosidl_runtime_rs/src/traits.rs b/rosidl_runtime_rs/src/traits.rs index 4737bd1aa..54764570a 100644 --- a/rosidl_runtime_rs/src/traits.rs +++ b/rosidl_runtime_rs/src/traits.rs @@ -24,7 +24,7 @@ use std::fmt::Debug; /// User code never needs to call these trait methods, much less implement this trait. pub trait SequenceAlloc: Sized { /// Wraps the corresponding init function generated by `rosidl_generator_c`. - fn sequence_init(seq: &mut crate::Sequence, size: libc::size_t) -> bool; + fn sequence_init(seq: &mut crate::Sequence, size: usize) -> bool; /// Wraps the corresponding fini function generated by `rosidl_generator_c`. fn sequence_fini(seq: &mut crate::Sequence); /// Wraps the corresponding copy function generated by `rosidl_generator_c`. @@ -42,7 +42,7 @@ pub trait RmwMessage: Clone + Debug + Default + Send + Sync + Message { const TYPE_NAME: &'static str; /// Get a pointer to the correct `rosidl_message_type_support_t` structure. - fn get_type_support() -> libc::uintptr_t; + fn get_type_support() -> usize; } /// Trait for types that can be used in a `rclrs::Subscription` and a `rclrs::Publisher`. @@ -158,5 +158,5 @@ pub trait Service: 'static { type Response: Message; /// Get a pointer to the correct `rosidl_service_type_support_t` structure. - fn get_type_support() -> libc::uintptr_t; + fn get_type_support() -> usize; } From 0501d54f3b47de97ba875caeef4d5e886fe2e76e Mon Sep 17 00:00:00 2001 From: Tatsuro Sakaguchi Date: Sun, 16 Oct 2022 03:02:48 +0000 Subject: [PATCH 4/9] Remove realloc and memcpy that are no longer needed --- rosidl_generator_rs/resource/msg_rmw.rs.em | 6 +-- rosidl_runtime_rs/src/sequence.rs | 50 +++------------------- rosidl_runtime_rs/src/string.rs | 9 ++-- 3 files changed, 15 insertions(+), 50 deletions(-) diff --git a/rosidl_generator_rs/resource/msg_rmw.rs.em b/rosidl_generator_rs/resource/msg_rmw.rs.em index 3339bd4c2..111a1c87d 100644 --- a/rosidl_generator_rs/resource/msg_rmw.rs.em +++ b/rosidl_generator_rs/resource/msg_rmw.rs.em @@ -27,6 +27,7 @@ extern "C" { fn @(package_name)__@(subfolder)__@(type_name)__init(msg: *mut @(type_name)) -> bool; fn @(package_name)__@(subfolder)__@(type_name)__Sequence__init(seq: *mut rosidl_runtime_rs::Sequence<@(type_name)>, size: usize) -> bool; fn @(package_name)__@(subfolder)__@(type_name)__Sequence__fini(seq: *mut rosidl_runtime_rs::Sequence<@(type_name)>); + fn @(package_name)__@(subfolder)__@(type_name)__Sequence__copy(in_seq: &rosidl_runtime_rs::Sequence<@(type_name)>, out_seq: *mut rosidl_runtime_rs::Sequence<@(type_name)>) -> bool; } @# Drop is not needed, since the default drop glue does the same as fini here: @@ -89,9 +90,8 @@ impl rosidl_runtime_rs::SequenceAlloc for @(type_name) { unsafe { @(package_name)__@(subfolder)__@(type_name)__Sequence__fini(seq as *mut _) } } fn sequence_copy(in_seq: &rosidl_runtime_rs::Sequence, out_seq: &mut rosidl_runtime_rs::Sequence) -> bool { - out_seq.resize_to_at_least(in_seq.len()); - out_seq.clone_from_slice(in_seq.as_slice()); - true + // SAFETY: This is safe since a the point is guaranteed to be valid/initialized. + unsafe { @(package_name)__@(subfolder)__@(type_name)__Sequence__copy(in_seq, out_seq as *mut _) } } } diff --git a/rosidl_runtime_rs/src/sequence.rs b/rosidl_runtime_rs/src/sequence.rs index db38704f0..3ac2127b8 100644 --- a/rosidl_runtime_rs/src/sequence.rs +++ b/rosidl_runtime_rs/src/sequence.rs @@ -274,30 +274,6 @@ where } } -impl Sequence { - /// Internal function for the sequence_copy impl. To be removed when rosidl#650 is backported and released. - pub fn resize_to_at_least(&mut self, len: usize) { - let allocation_size = std::mem::size_of::() * len; - if self.capacity < len { - // SAFETY: The memory in self.data is owned by C. - let data = unsafe { libc::realloc(self.data as *mut _, allocation_size) } as *mut T; - if data.is_null() { - panic!("realloc failed"); - } - // Initialize the new memory - for i in self.capacity..len { - // SAFETY: i is in bounds, and write() is appropriate for initializing uninitialized memory - unsafe { - data.add(i).write(T::default()); - } - } - self.data = data; - self.size = len; - self.capacity = len; - } - } -} - // ========================= impl for BoundedSequence ========================= impl Debug for BoundedSequence { @@ -520,6 +496,10 @@ macro_rules! impl_sequence_alloc_for_primitive_type { extern "C" { fn $init_func(seq: *mut Sequence<$rust_type>, size: usize) -> bool; fn $fini_func(seq: *mut Sequence<$rust_type>); + fn $copy_func( + in_seq: *const Sequence<$rust_type>, + out_seq: *mut Sequence<$rust_type>, + ) -> bool; } impl SequenceAlloc for $rust_type { @@ -538,26 +518,8 @@ macro_rules! impl_sequence_alloc_for_primitive_type { unsafe { $fini_func(seq as *mut _) } } fn sequence_copy(in_seq: &Sequence, out_seq: &mut Sequence) -> bool { - let allocation_size = std::mem::size_of::() * in_seq.size; - if out_seq.capacity < in_seq.size { - // SAFETY: The memory in out_seq.data is owned by C. - let data = unsafe { libc::realloc(out_seq.data as *mut _, allocation_size) }; - if data.is_null() { - return false; - } - out_seq.data = data as *mut _; - out_seq.capacity = in_seq.size; - } - // SAFETY: The memory areas don't overlap. - unsafe { - libc::memcpy( - out_seq.data as *mut _, - in_seq.data as *const _, - allocation_size, - ); - } - out_seq.size = in_seq.size; - true + // SAFETY: There are no special preconditions to the sequence_copy function. + unsafe { $copy_func(in_seq as *const _, out_seq as *mut _) } } } }; diff --git a/rosidl_runtime_rs/src/string.rs b/rosidl_runtime_rs/src/string.rs index 87b75442e..75f1ec7a5 100644 --- a/rosidl_runtime_rs/src/string.rs +++ b/rosidl_runtime_rs/src/string.rs @@ -120,6 +120,10 @@ macro_rules! string_impl { fn $assignn(s: *mut $string, value: *const $char_type, n: usize) -> bool; fn $sequence_init(seq: *mut Sequence<$string>, size: usize) -> bool; fn $sequence_fini(seq: *mut Sequence<$string>); + fn $sequence_copy( + in_seq: *const Sequence<$string>, + out_seq: *mut Sequence<$string>, + ) -> bool; } impl Default for $string { @@ -235,9 +239,8 @@ macro_rules! string_impl { unsafe { $sequence_fini(seq as *mut _) } } fn sequence_copy(in_seq: &Sequence, out_seq: &mut Sequence) -> bool { - out_seq.resize_to_at_least(in_seq.len()); - out_seq.clone_from_slice(in_seq.as_slice()); - true + // SAFETY: There are no special preconditions to the sequence_copy function. + unsafe { $sequence_copy(in_seq as *const _, out_seq as *mut _) } } } }; From b6c8abccc6e994ff427509af0f648ec0e825a529 Mon Sep 17 00:00:00 2001 From: Tatsuro Sakaguchi Date: Sun, 16 Oct 2022 02:50:55 +0000 Subject: [PATCH 5/9] Remove libc dependencies --- rclrs/Cargo.toml | 2 -- rosidl_runtime_rs/Cargo.toml | 3 --- 2 files changed, 5 deletions(-) diff --git a/rclrs/Cargo.toml b/rclrs/Cargo.toml index 00a9c09b1..50c9c3334 100644 --- a/rclrs/Cargo.toml +++ b/rclrs/Cargo.toml @@ -14,8 +14,6 @@ path = "src/lib.rs" # Please keep the list of dependencies alphabetically sorted, # and also state why each dependency is needed. [dependencies] -# Needed for FFI -libc = "0.2.43" # Needed for the Message trait, among others rosidl_runtime_rs = "0.3" # Needed for clients diff --git a/rosidl_runtime_rs/Cargo.toml b/rosidl_runtime_rs/Cargo.toml index 1897650fe..cb71252f9 100644 --- a/rosidl_runtime_rs/Cargo.toml +++ b/rosidl_runtime_rs/Cargo.toml @@ -13,8 +13,6 @@ path = "src/lib.rs" # Please keep the list of dependencies alphabetically sorted, # and also state why each dependency is needed. [dependencies] -# Needed for FFI -libc = "0.2" # Optional dependency for making it possible to convert messages to and from # formats such as JSON, YAML, Pickle, etc. serde = { version = "1", optional = true } @@ -24,4 +22,3 @@ serde = { version = "1", optional = true } quickcheck = "1" # Needed for testing serde support serde_json = "1" - From 44336c271a5cb8df43dbb8a3d85fc91f9f0d1b18 Mon Sep 17 00:00:00 2001 From: Tatsuro Sakaguchi Date: Mon, 17 Oct 2022 21:09:49 +0900 Subject: [PATCH 6/9] Switch to std::os::raw for support versions prior to 1.64.0 --- rclrs/src/arguments.rs | 8 ++++---- rclrs/src/context.rs | 2 +- rclrs/src/node.rs | 2 +- rclrs/src/node/graph.rs | 14 +++++++------- rclrs/src/parameter/override_map.rs | 2 +- rclrs/src/subscription/message_info.rs | 2 +- rosidl_runtime_rs/src/string.rs | 10 +++++----- 7 files changed, 20 insertions(+), 20 deletions(-) diff --git a/rclrs/src/arguments.rs b/rclrs/src/arguments.rs index 1d8b8e301..c091bf759 100644 --- a/rclrs/src/arguments.rs +++ b/rclrs/src/arguments.rs @@ -1,6 +1,6 @@ -use std::ffi::c_char; -use std::ffi::c_void; use std::ffi::CString; +use std::os::raw::c_char; +use std::os::raw::c_void; use std::ptr::null_mut; use crate::error::*; @@ -88,11 +88,11 @@ pub fn extract_non_ros_args( /// `rcl_arguments_get_count_unparsed_ros` -> `rcl_arguments_get_count_ros` /// ... pub(crate) fn get_rcl_arguments( - rcl_get_count: unsafe extern "C" fn(*const rcl_arguments_t) -> std::ffi::c_int, + rcl_get_count: unsafe extern "C" fn(*const rcl_arguments_t) -> std::os::raw::c_int, rcl_get_indices: unsafe extern "C" fn( *const rcl_arguments_t, rcl_allocator_t, - *mut *mut std::ffi::c_int, + *mut *mut std::os::raw::c_int, ) -> rcl_ret_t, rcl_arguments: *const rcl_arguments_t, args: &[String], diff --git a/rclrs/src/context.rs b/rclrs/src/context.rs index 88104c399..83ef17c56 100644 --- a/rclrs/src/context.rs +++ b/rclrs/src/context.rs @@ -1,5 +1,5 @@ -use std::ffi::c_char; use std::ffi::CString; +use std::os::raw::c_char; use std::string::String; use std::sync::{Arc, Mutex}; use std::vec::Vec; diff --git a/rclrs/src/node.rs b/rclrs/src/node.rs index 8d85d85f9..cc0ecc665 100644 --- a/rclrs/src/node.rs +++ b/rclrs/src/node.rs @@ -1,9 +1,9 @@ mod builder; mod graph; use std::cmp::PartialEq; -use std::ffi::c_char; use std::ffi::CStr; use std::fmt; +use std::os::raw::c_char; use std::sync::{Arc, Mutex, Weak}; use std::vec::Vec; diff --git a/rclrs/src/node/graph.rs b/rclrs/src/node/graph.rs index fdfe3c7ac..cacf96837 100644 --- a/rclrs/src/node/graph.rs +++ b/rclrs/src/node/graph.rs @@ -68,8 +68,8 @@ impl Node { unsafe extern "C" fn wrapper( node: *const rcl_node_t, allocator: *mut rcl_allocator_t, - node_name: *const ::std::ffi::c_char, - node_namespace: *const ::std::ffi::c_char, + node_name: *const ::std::os::raw::c_char, + node_namespace: *const ::std::os::raw::c_char, topic_names_and_types: *mut rcl_names_and_types_t, ) -> rcl_ret_t { rcl_get_publisher_names_and_types_by_node( @@ -95,8 +95,8 @@ impl Node { unsafe extern "C" fn wrapper( node: *const rcl_node_t, allocator: *mut rcl_allocator_t, - node_name: *const ::std::ffi::c_char, - node_namespace: *const ::std::ffi::c_char, + node_name: *const ::std::os::raw::c_char, + node_namespace: *const ::std::os::raw::c_char, topic_names_and_types: *mut rcl_names_and_types_t, ) -> rcl_ret_t { rcl_get_subscriber_names_and_types_by_node( @@ -316,8 +316,8 @@ impl Node { getter: unsafe extern "C" fn( *const rcl_node_t, *mut rcl_allocator_t, - *const ::std::ffi::c_char, - *const ::std::ffi::c_char, + *const ::std::os::raw::c_char, + *const ::std::os::raw::c_char, *mut rcl_names_and_types_t, ) -> rcl_ret_t, ) -> Result { @@ -355,7 +355,7 @@ impl Node { getter: unsafe extern "C" fn( *const rcl_node_t, *mut rcl_allocator_t, - *const ::std::ffi::c_char, + *const ::std::os::raw::c_char, bool, *mut rcl_topic_endpoint_info_array_t, ) -> rcl_ret_t, diff --git a/rclrs/src/parameter/override_map.rs b/rclrs/src/parameter/override_map.rs index 868008813..72c3e70c4 100644 --- a/rclrs/src/parameter/override_map.rs +++ b/rclrs/src/parameter/override_map.rs @@ -1,6 +1,6 @@ use std::collections::BTreeMap; -use std::ffi::c_char; use std::ffi::CStr; +use std::os::raw::c_char; use crate::rcl_bindings::*; use crate::{ParameterValue, RclrsError, ToResult}; diff --git a/rclrs/src/subscription/message_info.rs b/rclrs/src/subscription/message_info.rs index b2e08cf47..b1dae04cb 100644 --- a/rclrs/src/subscription/message_info.rs +++ b/rclrs/src/subscription/message_info.rs @@ -37,7 +37,7 @@ pub struct PublisherGid { /// do not need it. /// /// [1]: std::ffi::CString - pub implementation_identifier: *const std::ffi::c_char, + pub implementation_identifier: *const std::os::raw::c_char, } // SAFETY: The implementation identifier doesn't care about which thread it's read from. diff --git a/rosidl_runtime_rs/src/string.rs b/rosidl_runtime_rs/src/string.rs index 75f1ec7a5..201889a95 100644 --- a/rosidl_runtime_rs/src/string.rs +++ b/rosidl_runtime_rs/src/string.rs @@ -29,7 +29,7 @@ use crate::traits::SequenceAlloc; pub struct String { /// Dynamic memory in this type is allocated and deallocated by C, but this is a detail that is managed by /// the relevant functions and trait impls. - data: *mut std::ffi::c_char, + data: *mut std::os::raw::c_char, size: usize, capacity: usize, } @@ -50,7 +50,7 @@ pub struct String { /// ``` #[repr(C)] pub struct WString { - data: *mut std::ffi::c_ushort, + data: *mut std::os::raw::c_ushort, size: usize, capacity: usize, } @@ -248,7 +248,7 @@ macro_rules! string_impl { string_impl!( String, - std::ffi::c_char, + std::os::raw::c_char, u8, from_utf8_lossy, rosidl_runtime_c__String__init, @@ -260,7 +260,7 @@ string_impl!( ); string_impl!( WString, - std::ffi::c_ushort, + std::os::raw::c_ushort, u16, from_utf16_lossy, rosidl_runtime_c__U16String__init, @@ -333,7 +333,7 @@ impl Debug for BoundedString { } impl Deref for BoundedString { - type Target = [std::ffi::c_char]; + type Target = [std::os::raw::c_char]; fn deref(&self) -> &Self::Target { self.inner.deref() } From dc041ffb2604c4fef59d73dc9b5d9710a033258a Mon Sep 17 00:00:00 2001 From: Tatsuro Sakaguchi Date: Tue, 18 Oct 2022 19:58:19 +0900 Subject: [PATCH 7/9] Fix comments --- rosidl_generator_rs/resource/msg_rmw.rs.em | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rosidl_generator_rs/resource/msg_rmw.rs.em b/rosidl_generator_rs/resource/msg_rmw.rs.em index 111a1c87d..6ee871b6b 100644 --- a/rosidl_generator_rs/resource/msg_rmw.rs.em +++ b/rosidl_generator_rs/resource/msg_rmw.rs.em @@ -82,15 +82,15 @@ impl Default for @(type_name) { impl rosidl_runtime_rs::SequenceAlloc for @(type_name) { fn sequence_init(seq: &mut rosidl_runtime_rs::Sequence, size: usize) -> bool { - // SAFETY: This is safe since a the point is guaranteed to be valid/initialized. + // SAFETY: This is safe since the pointer is guaranteed to be valid/initialized. unsafe { @(package_name)__@(subfolder)__@(type_name)__Sequence__init(seq as *mut _, size) } } fn sequence_fini(seq: &mut rosidl_runtime_rs::Sequence) { - // SAFETY: This is safe since a the point is guaranteed to be valid/initialized. + // SAFETY: This is safe since the pointer is guaranteed to be valid/initialized. unsafe { @(package_name)__@(subfolder)__@(type_name)__Sequence__fini(seq as *mut _) } } fn sequence_copy(in_seq: &rosidl_runtime_rs::Sequence, out_seq: &mut rosidl_runtime_rs::Sequence) -> bool { - // SAFETY: This is safe since a the point is guaranteed to be valid/initialized. + // SAFETY: This is safe since the pointer is guaranteed to be valid/initialized. unsafe { @(package_name)__@(subfolder)__@(type_name)__Sequence__copy(in_seq, out_seq as *mut _) } } } From f7f8a43c32c81235f3e1bca74b9a7143002bd3d1 Mon Sep 17 00:00:00 2001 From: Tatsuro Sakaguchi Date: Tue, 18 Oct 2022 20:00:15 +0900 Subject: [PATCH 8/9] Return value of get_type_support() to *const c_void --- rosidl_generator_rs/resource/msg_rmw.rs.em | 4 ++-- rosidl_generator_rs/resource/srv.rs.em | 8 ++++---- rosidl_runtime_rs/src/traits.rs | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/rosidl_generator_rs/resource/msg_rmw.rs.em b/rosidl_generator_rs/resource/msg_rmw.rs.em index 6ee871b6b..b85061815 100644 --- a/rosidl_generator_rs/resource/msg_rmw.rs.em +++ b/rosidl_generator_rs/resource/msg_rmw.rs.em @@ -19,7 +19,7 @@ type_name = msg_spec.structure.namespaced_type.name #[link(name = "@(package_name)__rosidl_typesupport_c")] extern "C" { - fn rosidl_typesupport_c__get_message_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> usize; + fn rosidl_typesupport_c__get_message_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> *const std::os::raw::c_void; } #[link(name = "@(package_name)__rosidl_generator_c")] @@ -103,7 +103,7 @@ impl rosidl_runtime_rs::Message for @(type_name) { impl rosidl_runtime_rs::RmwMessage for @(type_name) where Self: Sized { const TYPE_NAME: &'static str = "@(package_name)/@(subfolder)/@(type_name)"; - fn get_type_support() -> usize { + fn get_type_support() -> *const std::os::raw::c_void { // SAFETY: No preconditions for this function. unsafe { rosidl_typesupport_c__get_message_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() } } diff --git a/rosidl_generator_rs/resource/srv.rs.em b/rosidl_generator_rs/resource/srv.rs.em index 00d0a409f..31cabc768 100644 --- a/rosidl_generator_rs/resource/srv.rs.em +++ b/rosidl_generator_rs/resource/srv.rs.em @@ -24,7 +24,7 @@ type_name = srv_spec.namespaced_type.name #[link(name = "@(package_name)__rosidl_typesupport_c")] extern "C" { - fn rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> usize; + fn rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> *const std::os::raw::c_void; } // Corresponds to @(package_name)__@(subfolder)__@(type_name) @@ -34,7 +34,7 @@ impl rosidl_runtime_rs::Service for @(type_name) { type Request = crate::@(subfolder)::@(type_name)_Request; type Response = crate::@(subfolder)::@(type_name)_Response; - fn get_type_support() -> usize { + fn get_type_support() -> *const std::os::raw::c_void { // SAFETY: No preconditions for this function. unsafe { rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() } } @@ -61,7 +61,7 @@ type_name = srv_spec.namespaced_type.name #[link(name = "@(package_name)__rosidl_typesupport_c")] extern "C" { - fn rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> usize; + fn rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() -> *const std::os::raw::c_void; } // Corresponds to @(package_name)__@(subfolder)__@(type_name) @@ -71,7 +71,7 @@ type_name = srv_spec.namespaced_type.name type Request = crate::@(subfolder)::rmw::@(type_name)_Request; type Response = crate::@(subfolder)::rmw::@(type_name)_Response; - fn get_type_support() -> usize { + fn get_type_support() -> *const std::os::raw::c_void { // SAFETY: No preconditions for this function. unsafe { rosidl_typesupport_c__get_service_type_support_handle__@(package_name)__@(subfolder)__@(type_name)() } } diff --git a/rosidl_runtime_rs/src/traits.rs b/rosidl_runtime_rs/src/traits.rs index 54764570a..d468a42d5 100644 --- a/rosidl_runtime_rs/src/traits.rs +++ b/rosidl_runtime_rs/src/traits.rs @@ -42,7 +42,7 @@ pub trait RmwMessage: Clone + Debug + Default + Send + Sync + Message { const TYPE_NAME: &'static str; /// Get a pointer to the correct `rosidl_message_type_support_t` structure. - fn get_type_support() -> usize; + fn get_type_support() -> *const std::os::raw::c_void; } /// Trait for types that can be used in a `rclrs::Subscription` and a `rclrs::Publisher`. @@ -158,5 +158,5 @@ pub trait Service: 'static { type Response: Message; /// Get a pointer to the correct `rosidl_service_type_support_t` structure. - fn get_type_support() -> usize; + fn get_type_support() -> *const std::os::raw::c_void; } From a25bee7ba3ed036960bdc02ab9504a94bedd9726 Mon Sep 17 00:00:00 2001 From: Tatsuro Sakaguchi Date: Tue, 18 Oct 2022 20:00:26 +0900 Subject: [PATCH 9/9] Fix dependency --- rosidl_generator_rs/resource/Cargo.toml.em | 1 - 1 file changed, 1 deletion(-) diff --git a/rosidl_generator_rs/resource/Cargo.toml.em b/rosidl_generator_rs/resource/Cargo.toml.em index 4c730bf95..110d2c41d 100644 --- a/rosidl_generator_rs/resource/Cargo.toml.em +++ b/rosidl_generator_rs/resource/Cargo.toml.em @@ -4,7 +4,6 @@ version = "@(package_version)" edition = "2021" [dependencies] -libc = "0.2" rosidl_runtime_rs = "0.3" serde = { version = "1", optional = true, features = ["derive"] } @[for dep in dependency_packages]@