Skip to content

uucore: replace unsafe mkfifo wrapper with nix crate implementation#11249

Open
mattsu2020 wants to merge 1 commit intouutils:mainfrom
mattsu2020:mkinfo
Open

uucore: replace unsafe mkfifo wrapper with nix crate implementation#11249
mattsu2020 wants to merge 1 commit intouutils:mainfrom
mattsu2020:mkinfo

Conversation

@mattsu2020
Copy link
Contributor

Replace the unsafe libc::mkfifo wrapper with a safe implementation using the nix crate. This change improves code safety by eliminating unsafe code blocks and leveraging the well-tested nix crate for system calls. The functionality remains identical - creating named pipes with 0o666 permissions - but now uses a safer, more idiomatic Rust approach.

Replace the unsafe `libc::mkfifo` wrapper with a safe implementation using the `nix` crate. This change improves code safety by eliminating unsafe code blocks and leveraging the well-tested nix crate for system calls. The functionality remains identical - creating named pipes with 0o666 permissions - but now uses a safer, more idiomatic Rust approach.
@github-actions
Copy link

github-actions bot commented Mar 9, 2026

GNU testsuite comparison:

Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/pr/bounded-memory (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/printf/printf-surprise is now being skipped but was previously passing.
Congrats! The gnu test tests/csplit/csplit-heap is now passing!
Congrats! The gnu test tests/rm/many-dir-entries-vs-OOM is now passing!

@cakebaker
Copy link
Contributor

I don't know if you have seen it: the redox job in the CI currently fails with:

error[E0425]: cannot find function `mkfifo` in module `nix::unistd`
   --> src/uucore/src/lib/features/fs.rs:840:18
    |
840 |     nix::unistd::mkfifo(path, nix::sys::stat::Mode::from_bits_truncate(0o666))?;
    |                  ^^^^^^ not found in `nix::unistd`
    |
note: found an item that was configured out
   --> /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/nix-0.30.1/src/unistd.rs:808:8
    |
807 | #[cfg(not(target_os = "redox"))] // RedoxFS does not support fifo yet
    |          --------------------- the item is gated here

@xtqqczze
Copy link
Contributor

xtqqczze commented Mar 9, 2026

We should open a nix issue, it seems redoxos does in fact support mkfifo:

https://github.com/redox-os/relibc/blob/9340d76a92fa1c454ae0136d9d7d7f3a41bf4788/src/header/sys_stat/mod.rs#L190

@xtqqczze
Copy link
Contributor

xtqqczze commented Mar 9, 2026

I think we could remove the make_fifo wrapper and just use mkfifo(path, Mode::from_bits_truncate(0o666)) everywhere?

@cakebaker
Copy link
Contributor

@xtqqczze I opened a PR there to remove the gate: nix-rust/nix#2749

@cakebaker
Copy link
Contributor

I think we could remove the make_fifo wrapper and just use mkfifo(path, Mode::from_bits_truncate(0o666)) everywhere?

Yes, that's a possibility.

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.

3 participants