Unnecessary `unsafe` for `libc::major` and `libc::minor`? Or is `libc::makedev` missing unsafe?
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 2.6k
- Forks
- 1.3k
- Avg merge
- 1d 22h
- Merged PRs (30d)
- 69
Description
- The
libc::majorandlibc::minorfunctions are unsafe. The reverse operationlibc::makedevis 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
makedevis 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
makedevandmajor/minorin 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/minorare wrong. - Is the idea that functions should be unsafe if it they call C functions on any platforms? Then
makedevis wrong. I can see the argument to not get lints about unnecessary unsafe blocks, but that only works if it is done consistently.
- Is the idea that functions should only be unsafe on platforms where a C function is called? Then
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
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by comparing the platform implementations and signatures of libc::major, libc::minor, and libc::makedev, including the Illumos variants. Check the relevant Unix man pages and existing safety documentation. Done means determining whether the unsafe annotations are correct and documenting or consistently changing them across supported targets.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100