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

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

Open
VorpalBlade opened this issue Jun 24, 2024 · 0 comments
Labels
C-bug Category: bug

Comments

@VorpalBlade
Copy link

VorpalBlade commented Jun 24, 2024

  • 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

@VorpalBlade VorpalBlade added the C-bug Category: bug label Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

1 participant