Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions rclrs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions rclrs/src/arguments.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use std::ffi::CString;
use std::os::raw::c_char;
use std::os::raw::c_void;
use std::ptr::null_mut;

use libc::c_void;

use crate::error::*;
use crate::rcl_bindings::*;

Expand Down
2 changes: 1 addition & 1 deletion rclrs/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ mod graph;
use std::cmp::PartialEq;
use std::ffi::CStr;
use std::fmt;
use std::os::raw::c_char;
use std::sync::{Arc, Mutex, Weak};
use std::vec::Vec;

use libc::c_char;
use rosidl_runtime_rs::Message;

pub use self::builder::*;
Expand Down
3 changes: 1 addition & 2 deletions rclrs/src/parameter/override_map.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use std::collections::BTreeMap;
use std::ffi::CStr;

use libc::c_char;
use std::os::raw::c_char;

use crate::rcl_bindings::*;
use crate::{ParameterValue, RclrsError, ToResult};
Expand Down
14 changes: 7 additions & 7 deletions rosidl_generator_rs/resource/msg_rmw.rs.em
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,15 @@ 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)>);
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:
Expand Down Expand Up @@ -80,7 +81,7 @@ impl Default for @(type_name) {
}

impl rosidl_runtime_rs::SequenceAlloc for @(type_name) {
fn sequence_init(seq: &mut rosidl_runtime_rs::Sequence<Self>, size: libc::size_t) -> bool {
fn sequence_init(seq: &mut rosidl_runtime_rs::Sequence<Self>, 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) }
}
Expand All @@ -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<Self>, out_seq: &mut rosidl_runtime_rs::Sequence<Self>) -> 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.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// 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 _) }
}
}

Expand All @@ -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)() }
}
Expand Down
8 changes: 4 additions & 4 deletions rosidl_generator_rs/resource/srv.rs.em
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)() }
}
Expand All @@ -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)
Expand All @@ -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)() }
}
Expand Down
3 changes: 0 additions & 3 deletions rosidl_runtime_rs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand All @@ -24,4 +22,3 @@ serde = { version = "1", optional = true }
quickcheck = "1"
# Needed for testing serde support
serde_json = "1"

58 changes: 10 additions & 48 deletions rosidl_runtime_rs/src/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@ use crate::traits::SequenceAlloc;
#[repr(C)]
pub struct Sequence<T: SequenceAlloc> {
data: *mut T,
size: libc::size_t,
capacity: libc::size_t,
size: usize,
capacity: usize,
}

/// A bounded sequence.
Expand Down Expand Up @@ -274,30 +274,6 @@ where
}
}

impl<T: Default + SequenceAlloc> Sequence<T> {
/// 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::<Self>() * 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<T: Debug + SequenceAlloc, const N: usize> Debug for BoundedSequence<T, N> {
Expand Down Expand Up @@ -518,12 +494,16 @@ 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>);
fn $copy_func(
in_seq: *const Sequence<$rust_type>,
out_seq: *mut Sequence<$rust_type>,
) -> bool;
}

impl SequenceAlloc for $rust_type {
fn sequence_init(seq: &mut Sequence<Self>, size: libc::size_t) -> bool {
fn sequence_init(seq: &mut Sequence<Self>, 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
Expand All @@ -538,26 +518,8 @@ macro_rules! impl_sequence_alloc_for_primitive_type {
unsafe { $fini_func(seq as *mut _) }
}
fn sequence_copy(in_seq: &Sequence<Self>, out_seq: &mut Sequence<Self>) -> bool {
let allocation_size = std::mem::size_of::<Self>() * 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 _) }
}
}
};
Expand Down
37 changes: 20 additions & 17 deletions rosidl_runtime_rs/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ 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,
size: libc::size_t,
capacity: libc::size_t,
data: *mut std::os::raw::c_char,
size: usize,
capacity: usize,
}

/// A zero-terminated string of 16-bit characters.
Expand All @@ -50,9 +50,9 @@ pub struct String {
/// ```
#[repr(C)]
pub struct WString {
data: *mut libc::c_ushort,
size: libc::size_t,
capacity: libc::size_t,
data: *mut std::os::raw::c_ushort,
size: usize,
capacity: usize,
}

/// A zero-terminated string of 8-bit characters with a length limit.
Expand Down Expand Up @@ -117,9 +117,13 @@ 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>);
fn $sequence_copy(
in_seq: *const Sequence<$string>,
out_seq: *mut Sequence<$string>,
) -> bool;
}

impl Default for $string {
Expand Down Expand Up @@ -226,7 +230,7 @@ macro_rules! string_impl {
unsafe impl Sync for $string {}

impl SequenceAlloc for $string {
fn sequence_init(seq: &mut Sequence<Self>, size: libc::size_t) -> bool {
fn sequence_init(seq: &mut Sequence<Self>, size: usize) -> bool {
// SAFETY: There are no special preconditions to the sequence_init function.
unsafe { $sequence_init(seq as *mut _, size) }
}
Expand All @@ -235,17 +239,16 @@ macro_rules! string_impl {
unsafe { $sequence_fini(seq as *mut _) }
}
fn sequence_copy(in_seq: &Sequence<Self>, out_seq: &mut Sequence<Self>) -> 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 _) }
}
}
};
}

string_impl!(
String,
libc::c_char,
std::os::raw::c_char,
u8,
from_utf8_lossy,
rosidl_runtime_c__String__init,
Expand All @@ -257,7 +260,7 @@ string_impl!(
);
string_impl!(
WString,
libc::c_ushort,
std::os::raw::c_ushort,
u16,
from_utf16_lossy,
rosidl_runtime_c__U16String__init,
Expand Down Expand Up @@ -330,7 +333,7 @@ impl<const N: usize> Debug for BoundedString<N> {
}

impl<const N: usize> Deref for BoundedString<N> {
type Target = [libc::c_char];
type Target = [std::os::raw::c_char];
fn deref(&self) -> &Self::Target {
self.inner.deref()
}
Expand All @@ -349,7 +352,7 @@ impl<const N: usize> Display for BoundedString<N> {
}

impl<const N: usize> SequenceAlloc for BoundedString<N> {
fn sequence_init(seq: &mut Sequence<Self>, size: libc::size_t) -> bool {
fn sequence_init(seq: &mut Sequence<Self>, 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<Self> as *mut _, size)
Expand Down Expand Up @@ -415,7 +418,7 @@ impl<const N: usize> Display for BoundedWString<N> {
}

impl<const N: usize> SequenceAlloc for BoundedWString<N> {
fn sequence_init(seq: &mut Sequence<Self>, size: libc::size_t) -> bool {
fn sequence_init(seq: &mut Sequence<Self>, 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<Self> as *mut _, size)
Expand Down
6 changes: 3 additions & 3 deletions rosidl_runtime_rs/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Self>, size: libc::size_t) -> bool;
fn sequence_init(seq: &mut crate::Sequence<Self>, size: usize) -> bool;
/// Wraps the corresponding fini function generated by `rosidl_generator_c`.
fn sequence_fini(seq: &mut crate::Sequence<Self>);
/// Wraps the corresponding copy function generated by `rosidl_generator_c`.
Expand All @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we're here – could we make this pointer and the one in the Service trait a *const c_void?

}

/// Trait for types that can be used in a `rclrs::Subscription` and a `rclrs::Publisher`.
Expand Down Expand Up @@ -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;
}