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

Replace the Fcntl enum with values from libc #70

Closed
asomers opened this issue Jan 22, 2024 · 2 comments · Fixed by #87
Closed

Replace the Fcntl enum with values from libc #70

asomers opened this issue Jan 22, 2024 · 2 comments · Fixed by #87

Comments

@asomers
Copy link
Collaborator

asomers commented Jan 22, 2024

Replace Fcntl::GetFL with libc::F_GETFL, etc.

@asomers
Copy link
Collaborator Author

asomers commented Jan 23, 2024

We have a decision to make here. The tradeoff is type safety vs future proofing. The most future-proof way to do this would be to accept any u32 in FcntlsBuilder::add. That's similar to what IoctlsBuilder does. But it isn't type-safe, which is unfortunate. A safer option would be to create an enum of allowed values. We could even create it automatically by copying Nix's libc_enum! macro. But that isn't future-proof, because if FreeBSD adds a new fnctl value, capsicum-rs would need to be updated before our users could make use.
A compromise might be to define Fcntl values as consts, like this:

struct Fcntl(u32);
impl Fcntl {
    const F_GETFL = Fcntl(libc::F_GETFL);
    ...
}

Or something along those lines. What do you think?

The same argument applies to IoctlsBuilder. However, allowed ioctl values are much more numerous, and they may not all be defined in libc. Plus, users may even have custom ioctls that come from out-of-tree (or even proprietary) kernel modules. So for IoctlsBuilder, future-proofing is much more important.

@asomers
Copy link
Collaborator Author

asomers commented Mar 22, 2024

Depends on rust-lang/libc#3628

asomers added a commit to asomers/capsicum-rs that referenced this issue Mar 22, 2024
asomers added a commit to asomers/capsicum-rs that referenced this issue Mar 22, 2024
asomers added a commit to asomers/capsicum-rs that referenced this issue Oct 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant