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

libc types exposed in public API #4916

Closed
jdisanti opened this issue Aug 16, 2022 · 2 comments · Fixed by #5191
Closed

libc types exposed in public API #4916

jdisanti opened this issue Aug 16, 2022 · 2 comments · Fixed by #5191
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-net Module: tokio/net

Comments

@jdisanti
Copy link
Contributor

As discovered in #4914, the net module exposes libc::gid_t, libc::pid_t, and libc::uid_t in getter functions on the UCred struct. These types are all type aliases to primitives in libc:

https://github.com/rust-lang/libc/blob/08c0f2c00a5dfa10a9fbc74150b7dbecf2164682/src/unix/mod.rs#L25-L39

I'm creating this issue to discuss the best path forward for addressing it and track its fix.

I can think of a couple approaches that could be taken:

  1. Tokio provides its own gid_t, pid_t, and uid_t aliases in net::unix, and copies libc's target OS config to determine their shape. This would be fragile since libc changes these type aliases based on the target OS. Today, libc only changes them between u32/u16 based on the espidf and horizon target OS, but more configurations could be added over time and become out of sync with Tokio.
  2. Tokio provides its own gid_t, pid_t, and uid_t aliases in net::unix, but sets them to u32, i32, and u32 respectively to match libc's default configuration. When Tokio adds support for target OSes where libc special cases the types, these smaller primitive values can be coerced into their larger equivalents in the platform-specific implementation of get_peer_cred. If libc adds a target OS that makes these types larger than its default config (e.g., `u64), and Tokio wants to add support for that OS, then the first approach would need to be taken at that time.

I think that approach 2 is probably the better choice since it allows Tokio to punt on adding cfg conditions for target OSes it may not want to add get_peer_cred support to at present, while still making it possible to add that support at a later time.

@jdisanti jdisanti added A-tokio Area: The main tokio crate C-bug Category: This is a bug. labels Aug 16, 2022
@Darksonn Darksonn added the M-net Module: tokio/net label Aug 16, 2022
@Darksonn
Copy link
Contributor

So, what you're saying is that currently, all of the aliases have a single fixed type except for the espidf and horizon target OS?

@jdisanti
Copy link
Contributor Author

That's right. On espidf and horizon, the gid_t and uid_t types become u16 instead of u32.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-bug Category: This is a bug. M-net Module: tokio/net
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants