Skip to content

mknod: replace nix::sys::stat with rustix + libc#12321

Open
mattsu2020 wants to merge 4 commits into
uutils:mainfrom
mattsu2020:mknod_rustix
Open

mknod: replace nix::sys::stat with rustix + libc#12321
mattsu2020 wants to merge 4 commits into
uutils:mainfrom
mattsu2020:mknod_rustix

Conversation

@mattsu2020
Copy link
Copy Markdown
Contributor

@mattsu2020 mattsu2020 commented May 16, 2026

Summary

  • Replace nix::sys::stat::{Mode, SFlag, mknod, umask} with rustix::fs::{Mode, FileType} and rustix::process::umask
  • Use libc::mknod directly for the mknod(2) syscall since rustix::fs::mknodat is unavailable on Apple targets
  • Introduce RAII UmaskGuard (pattern from mkdir) ensuring umask is restored on drop, even on panic — improving over the previous manual save/restore
  • Simplify MODE_RW_UGO from individual nix::libc::S_I* constants to 0o666 literal

Dependency changes

  • Removed: nix
  • Added: rustix (features: process, fs), libc

Test plan

  • cargo build -p uu_mknod — compiles cleanly
  • cargo clippy -p uu_mknod — no warnings
  • mknod /tmp/test_fifo p — creates FIFO with correct default mode (prw-r--r--, umask applied)
  • mknod -m 0600 /tmp/test_fifo2 p — creates FIFO with exact mode (prw-------, umask bypassed)
  • CI green on Linux (block/char device tests)
  • CI green on macOS

Replace `nix::sys::stat` usage with `rustix::fs` types and `libc::mknod`:

- `nix::sys::stat::Mode` → `rustix::fs::Mode`
- `nix::sys::stat::SFlag` → `rustix::fs::FileType`
- `nix::sys::stat::umask` → `rustix::process::umask` with RAII `UmaskGuard`
- `nix::sys::stat::mknod` → `libc::mknod` (rustix::fs::mknodat is unavailable on apple targets)
- `nix::libc::S_I*` constants → `0o666` literal

The `UmaskGuard` pattern (borrowed from mkdir) ensures umask is restored
on drop, even on panic — an improvement over the previous manual save/restore.
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 16, 2026

GNU testsuite comparison:

Skipping an intermittent issue tests/cut/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/retry (passes in this run but fails in the 'main' branch)
Congrats! The gnu test tests/pr/bounded-memory is now passing!

Comment thread src/uu/mknod/src/mknod.rs Outdated

impl FileType {
fn as_sflag(&self) -> SFlag {
fn to_rustix(self) -> RustixFileType {

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix it

Comment thread src/uu/mknod/src/mknod.rs Outdated
uucore::execution_phrase(),
std::io::Error::from(err)
);
eprintln!("{}: {err}", uucore::execution_phrase());

This comment was marked as resolved.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix it

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented May 16, 2026

Merging this PR will improve performance by 13.71%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
✅ 315 untouched benchmarks
⏩ 46 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation ls_recursive_balanced_tree[(6, 4, 15)] 50.9 ms 48.9 ms +4.01%
Memory cp_recursive_deep_tree[(120, 4)] 699.2 KB 562.5 KB +24.31%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing mattsu2020:mknod_rustix (000ee9d) with main (d09f351)

Open in CodSpeed

Footnotes

  1. 46 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Comment thread src/uu/mknod/Cargo.toml
fluent = { workspace = true }
nix = { workspace = true }
rustix = { workspace = true, features = ["process", "fs"] }
libc = { workspace = true }

This comment was marked as resolved.

@oech3
Copy link
Copy Markdown
Contributor

oech3 commented May 16, 2026

mkfifo is subset of mknod. So it should be able to remake existing mkfifo wrapper without unsafe by this one.

- Rename `to_rustix` → `to_file_type` to avoid embedding crate name
- Use `let _ = writeln!(stderr, ...)` to prevent SIGABRT on /dev/full
- Sort Cargo.toml dependencies alphabetically (libc before rustix)
Replaced the multi-line writeln! macro call with a single-line version for better readability.
@mattsu2020
Copy link
Copy Markdown
Contributor Author

mkfifo is subset of mknod. So it should be able to remake existing mkfifo wrapper without unsafe by this one.

Thanks for the suggestion! You're right
mkfifo is essentially mknod with S_IFIFO and dev=0, so the do_mknod helper could be reused there to eliminate the nix dependency and unsafe as well.

@oech3
Copy link
Copy Markdown
Contributor

oech3 commented May 16, 2026

Is it able to open a PR on rustix to support rustix::fs::mk*at on macOS?
I have no access on macOS now...

@mattsu2020
Copy link
Copy Markdown
Contributor Author

Is it able to open a PR on rustix to support rustix::fs::mk*at on macOS? I have no access on macOS now...

I created the PR, but there are some errors on the Rustix side during CI, so I'll review the code again once the fixes are complete.
bytecodealliance/rustix#1625

@oech3
Copy link
Copy Markdown
Contributor

oech3 commented May 16, 2026

Thankyou

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 this pull request may close these issues.

2 participants