ros2_rust icon indicating copy to clipboard operation
ros2_rust copied to clipboard

Use std::os::raw types instead of libc crate

Open nnmm opened this issue 3 years ago • 3 comments
trafficstars

It seems that the libc crate is only really useful in #[no_std] environments. https://stackoverflow.com/questions/44436515/should-i-use-libcc-char-or-stdosrawc-char

We should see if we can replace all uses of libc with std::os::raw.

nnmm avatar Jul 04 '22 19:07 nnmm

I found libc crate use of the following items within this project.

  • c_void
    • It is used in *mut c_void, in which case both libc and std::os::raw are equivalent to C's void*.
  • c_char(Both libc and std are used.)
  • c_ushort
  • uintptr_t
  • realloc
    • Not implemented in std::os::raw. Compatibility with C is not specified, but std::alloc::realloc is the implementation in Rust.
  • memcpy
    • Not implemented in std::os::raw, but copy_nonoverlapping is semantically equivalent to C memcopy.
  • size_t
    • Not implemented in std::os::raw. However, since --size_t_is_usize is enabled in bindgen, it can be replaced by usize.(ref)

https://github.com/ros2-rust/ros2_rust/blob/0c262973b5abe0b8b66bca0285fae3ec59c0176f/rclrs/build.rs#L37

I consider c_void, c_char, c_ushort, uintptr_t to be fine, what about realloc, memcpy, size_t?

Tacha-S avatar Oct 15 '22 03:10 Tacha-S

I agree about replacing the integer aliases like you say. We could also use std::ffi instead of std::os::raw, I actually think that's a bit nicer.

For the allocation functions, they are currently only used in one place iirc: In sequence.rs, as a workaround for some functions that were missing in older distros. I had already backported them in https://github.com/ros2/rosidl/pull/667 but they hadn't landed in a Foxy release yet when I wrote sequence.rs. However, I just checked and they've now been released, so there is no need for the workaround anymore, and therefore for realloc and memcpy.

nnmm avatar Oct 15 '22 11:10 nnmm

Thank you for the background on realloc and memcpy.

I will create a draft with the replacement with std::ffi and the elimination of realloc and memcpy.

Tacha-S avatar Oct 16 '22 02:10 Tacha-S

Closed in #284

nnmm avatar Oct 19 '22 09:10 nnmm