Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
nnmm opened this issue Jul 4, 2022 · 4 comments
Closed

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

nnmm opened this issue Jul 4, 2022 · 4 comments

Comments

@nnmm
Copy link
Contributor

nnmm commented Jul 4, 2022

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.

@Tacha-S
Copy link
Contributor

Tacha-S commented Oct 15, 2022

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)

.size_t_is_usize(true)

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

@nnmm
Copy link
Contributor Author

nnmm commented Oct 15, 2022

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 ros2/rosidl#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.

@Tacha-S
Copy link
Contributor

Tacha-S commented Oct 16, 2022

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.

@nnmm
Copy link
Contributor Author

nnmm commented Oct 19, 2022

Closed in #284

@nnmm nnmm closed this as completed Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants