Skip to content

Unnecessary unsafe for libc::major and libc::minor? Or is libc::makedev missing unsafe? #3759

Open
@VorpalBlade

Description

@VorpalBlade
  • The libc::major and libc::minor functions are unsafe. The reverse operation libc::makedev is not.
  • There is no safety invariants on docs.rs for these, this seems to be the norm for libc, and I can see why (you should refer to the man page for the C safety invariants probably).
  • However, there is nothing in my (Linux Glibc) man page that describes why they would have safety invariants. Nor do the FreeBSD, NetBSD, OpenBSD or Illumos man pages have anything along those lines.
  • Most implementations are in pure rust and use no unsafe operations. The sole exception seems to be Illumos where C functions are being called. Here makedev is also unsafe, unlike other platforms.
  • Illumos can apparently return NODEV, but that is a safe failure (with a corresponding errno being set).

I have not been able to check the man page of every single Unix system, so maybe there is one where this makes sense. Otherwise I think this is an oversight.

I think one or both of the following should be done:

  • If it is not an oversight, since it is safe on the majority of Unices, it would be good to add a documentation string that explains why it is unsafe on some.
  • Also, I would expect some consistency between makedev and major/minor in how unsafe vs not unsafe is handled across platforms.
    • Is the idea that functions should only be unsafe on platforms where a C function is called? Then major/minor are wrong.
    • Is the idea that functions should be unsafe if it they call C functions on any platforms? Then makedev is wrong. I can see the argument to not get lints about unnecessary unsafe blocks, but that only works if it is done consistently.

I also very much doubt there is any Unix where any of these functions could reasonably be unsafe. After all, even the C function will only do some arithmetic.

Minimum working example (not that I think it makes sense here):

/// To be called with value from std::os::unix::fs::MetadataExt::rdev
fn split_to_major_minor(rdev: u64) -> (u64, u64) {
    // SAFETY: I don't think there *actually* are any safety invariants here
    (unsafe { libc::major(rdev) } as u64, unsafe { libc::minor(rdev) } as u64)
}

Target triplet: x86_64-unknown-linux-gnu (and several other ones I checked)
Libc version: 0.2.155

Metadata

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: bug

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions