Use bitflags crate for ... well ... bit flags...?

[nyurik]

libc has several bitflag type aliases, e.g. mode_t - which are currently not strongly typed, thus require users to constant deal with casting, and may cause some subtle bugs.

Would it make sense to introduce a nicer layer of abstraction over these fields using bitflags or similar crate? This will also help hiding the size of the actual OS-specific field - e.g. mode_t is u32 on most platforms, but it is i32 on Redox and u16 on some other OSes.

see also uutils/coreutils#7751

[tgross35]

I don't think we can do this exactly; libc needs to remain dependency-free by default so this would have to be a feature, but changing the type wouldn't be additive (one crate in the graph enabling bitflags would break others that rely on it not being there). I'm also not sure how strong the motivation in libc is if nix already exposes this.

It is possible that something nicer exists though, worth asking around on Zulip if you are interested.

[tgross35]

This also relates to a problem that a lot of our flags have; usually flags are an int in function signatures, but many times they are stored as short in structs. So Rust users have to convert at least once.

[nyurik]

Thx @tgross35, I actually think bitflags crate itself might be a poor fit, but rather libc could introduce its own simple bit flag macro, especially because libc flags need to be additive. Something simple like this (with some macro sugar):

#[cfg(target_os = "linux")]
type mode_t = u32;
#[cfg(target_os = "redox")]
type mode_t = i32;

#[repr(transparent)]
struct Mode(mode_t);

// file in src/unix
impl crate::Mode {
  const FLAG1 = Mode(0x1);
}

// file in src/unix/linux_like
impl crate::Mode {
  const FLAG2 = Mode(0x2);
}

I agree that we can't do much if a function requires any type other than mode_t for this case, but many functions do use mode_t - so they could in theory just take Mode value as it is transparent.

Note that I suspect Nix only does this because they would love to offload it to libc but can't. If libc starts doing it, this will make everyone's live much easier, esp because it can support all OS-es libc supports.

[nyurik]

P.S. The main issue with wrapping this functionality is that whatever wraps it must duplicate all the conditional logic that libc already has - so maintaining it seems like a huge and un-needed burden.

[nyurik]

P.P.S. I got a good answer from bitflags about it at bitflags/bitflags#441, so clearly bitflags won't work, but we can easily do much simpler libc-focused macro to generate what we need, and even have it in a backwards compatible way. This macro would expand to what we have in libc at the moment, as well as adds same constants to the Mode struct, which might be defined in another mod.

libc_bitflags! {
  Mode: mode_t => [
    // Note that these are identical to what libc has now, just moved inside the macro
    const S_IEXEC = crate::S_IXUSR;
    const S_IWRITE = crate::S_IWUSR;
    const S_IREAD = crate::S_IRUSR;
  ]
}