From 7ee112400631339a32516a648fe7c94c7ea6461a Mon Sep 17 00:00:00 2001 From: oech3 <79379754+oech3@users.noreply.github.com> Date: Tue, 12 May 2026 03:44:13 +0900 Subject: [PATCH] uucore: remove unsafe make_fifo --- Cargo.lock | 1 + src/uu/cp/src/cp.rs | 6 +-- src/uu/mv/Cargo.toml | 1 + src/uu/mv/src/mv.rs | 8 ++-- src/uucore/src/lib/features/fs.rs | 58 -------------------------- src/uucore/src/lib/features/selinux.rs | 4 +- 6 files changed, 11 insertions(+), 67 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a39a221006..bd7cce08b6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3815,6 +3815,7 @@ dependencies = [ "fs_extra", "indicatif", "libc", + "nix", "rustc-hash", "tempfile", "thiserror 2.0.18", diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 21adaabd9b..8f6ec6560b 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -29,8 +29,6 @@ use thiserror::Error; use platform::copy_on_write; use uucore::display::Quotable; use uucore::error::{UError, UResult, UUsageError, set_exit_code}; -#[cfg(unix)] -use uucore::fs::make_fifo; use uucore::fs::{ FileInformation, MissingHandling, ResolveMode, are_hardlinks_to_same_file, canonicalize, get_filename, is_symlink_loop, normalize_path, path_ends_with_terminator, @@ -2830,8 +2828,8 @@ fn copy_fifo(dest: &Path, overwrite: OverwriteMode, debug: bool) -> CopyResult<( overwrite.verify(dest, debug)?; fs::remove_file(dest)?; } - - make_fifo(dest) + // rustix::fs::mkfifoat is linux only + nix::unistd::mkfifo(dest, Mode::from_bits_truncate(0o666)) .map_err(|_| translate!("cp-error-cannot-create-fifo", "path" => dest.quote()).into()) } diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index ee0bc3b6a5..d83fb9d4e4 100644 --- a/src/uu/mv/Cargo.toml +++ b/src/uu/mv/Cargo.toml @@ -23,6 +23,7 @@ clap = { workspace = true } fs_extra = { workspace = true } indicatif = { workspace = true } libc = { workspace = true } +nix = { workspace = true } rustc-hash = { workspace = true } thiserror = { workspace = true } uucore = { workspace = true, features = [ diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index b750428f7e..8b93520d49 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -39,8 +39,6 @@ use uucore::display::Quotable; use uucore::error::{FromIo, UResult, USimpleError, UUsageError, set_exit_code}; #[cfg(unix)] use uucore::fs::display_permissions_unix; -#[cfg(unix)] -use uucore::fs::make_fifo; use uucore::fs::{ MissingHandling, ResolveMode, are_hardlinks_or_one_way_symlink_to_same_file, are_hardlinks_to_same_file, canonicalize, path_ends_with_terminator, @@ -885,7 +883,8 @@ fn rename_fifo_fallback(from: &Path, to: &Path) -> io::Result<()> { if to.try_exists()? { fs::remove_file(to)?; } - make_fifo(to)?; + // rustix::fs::mkfifoat is linux only + nix::unistd::mkfifo(to, nix::sys::stat::Mode::from_bits_truncate(0o666))?; fs::remove_file(from) } @@ -1162,7 +1161,8 @@ fn copy_file_with_hardlinks_helper( // rename_symlink_fallback already preserves ownership and removes the source. rename_symlink_fallback(from, to)?; } else if is_fifo(from.symlink_metadata()?.file_type()) { - make_fifo(to)?; + // rustix::fs::mkfifoat is linux only + nix::unistd::mkfifo(to, nix::sys::stat::Mode::from_bits_truncate(0o666))?; // Preserve ownership (uid/gid) from the source let _ = preserve_ownership(from, to); } else { diff --git a/src/uucore/src/lib/features/fs.rs b/src/uucore/src/lib/features/fs.rs index 9e9e9a8b8f..aece06bd60 100644 --- a/src/uucore/src/lib/features/fs.rs +++ b/src/uucore/src/lib/features/fs.rs @@ -7,15 +7,11 @@ // spell-checker:ignore backport -#[cfg(unix)] -use libc::mkfifo; #[cfg(all(unix, not(target_os = "redox")))] pub use libc::{major, makedev, minor}; use std::collections::HashSet; use std::collections::VecDeque; use std::env; -#[cfg(unix)] -use std::ffi::CString; use std::ffi::{OsStr, OsString}; use std::fs; use std::fs::read_dir; @@ -892,37 +888,6 @@ pub fn get_filename(file: &Path) -> Option<&str> { file.file_name().and_then(|filename| filename.to_str()) } -/// Make a FIFO, also known as a named pipe. -/// -/// This is a safe wrapper for the unsafe [`libc::mkfifo`] function, -/// which makes a [named -/// pipe](https://en.wikipedia.org/wiki/Named_pipe) on Unix systems. -/// -/// # Errors -/// -/// If the named pipe cannot be created. -/// -/// # Examples -/// -/// ```ignore -/// use uucore::fs::make_fifo; -/// -/// make_fifo("my-pipe").expect("failed to create the named pipe"); -/// -/// std::thread::spawn(|| { std::fs::write("my-pipe", b"hello").unwrap(); }); -/// assert_eq!(std::fs::read("my-pipe").unwrap(), b"hello"); -/// ``` -#[cfg(unix)] -pub fn make_fifo(path: &Path) -> std::io::Result<()> { - let name = CString::new(path.to_str().unwrap()).unwrap(); - let err = unsafe { mkfifo(name.as_ptr(), 0o666) }; - if err == -1 { - Err(Error::last_os_error()) - } else { - Ok(()) - } -} - // Redox's libc appears not to include the following utilities #[cfg(target_os = "redox")] @@ -950,8 +915,6 @@ mod tests { #[cfg(unix)] use std::os::unix; #[cfg(unix)] - use std::os::unix::fs::FileTypeExt; - #[cfg(unix)] use tempfile::{NamedTempFile, tempdir}; struct NormalizePathTestCase<'a> { @@ -1216,25 +1179,4 @@ mod tests { let file_path = PathBuf::from("~/foo.txt"); assert!(matches!(get_filename(&file_path), Some("foo.txt"))); } - - #[cfg(unix)] - #[test] - fn test_make_fifo() { - // Create the FIFO in a temporary directory. - let tempdir = tempdir().unwrap(); - let path = tempdir.path().join("f"); - assert!(make_fifo(&path).is_ok()); - - // Check that it is indeed a FIFO. - assert!(fs::metadata(&path).unwrap().file_type().is_fifo()); - - // Check that we can write to it and read from it. - // - // Write and read need to happen in different threads, - // otherwise `write` would block indefinitely while waiting - // for the `read`. - let path2 = path.clone(); - std::thread::spawn(move || assert!(fs::write(&path2, b"foo").is_ok())); - assert_eq!(fs::read(&path).unwrap(), b"foo"); - } } diff --git a/src/uucore/src/lib/features/selinux.rs b/src/uucore/src/lib/features/selinux.rs index 28c68a1b6c..b6a69dc9ec 100644 --- a/src/uucore/src/lib/features/selinux.rs +++ b/src/uucore/src/lib/features/selinux.rs @@ -618,7 +618,9 @@ mod tests { // Create a FIFO (pipe) let fifo_path = dir_path.join("my_fifo"); - crate::fs::make_fifo(&fifo_path).expect("Failed to create FIFO"); + // todo: use rustix::fs::mkfifoat since selinux is linux specific + nix::unistd::mkfifo(&fifo_path, nix::sys::stat::Mode::from_bits_truncate(0o666)) + .expect("Failed to create FIFO"); // Just getting a context is good enough get_selinux_security_context(&fifo_path, false).expect("Cannot get fifo context");