From 2717f9c8b54124957e478991d91febaa54c4f0d3 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Thu, 17 Apr 2025 20:32:34 -0400 Subject: [PATCH] mv: fix moving FIFO to a different filesystem Fix a bug in `mv` where it would hang indefinitely while trying to copy a FIFO across filesystems. The solution is to remove the old FIFO and create a new one on the new filesystem. Fixes #7076 --- src/uu/mv/Cargo.toml | 3 ++- src/uu/mv/src/mv.rs | 32 ++++++++++++++++++++++++++++++++ tests/by-util/test_mv.rs | 17 +++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) diff --git a/src/uu/mv/Cargo.toml b/src/uu/mv/Cargo.toml index 0b0d3da06..1cb30bf14 100644 --- a/src/uu/mv/Cargo.toml +++ b/src/uu/mv/Cargo.toml @@ -21,13 +21,14 @@ path = "src/mv.rs" clap = { workspace = true } fs_extra = { workspace = true } indicatif = { workspace = true } +libc = { workspace = true } +thiserror = { workspace = true } uucore = { workspace = true, features = [ "backup-control", "fs", "fsxattr", "update-control", ] } -thiserror = { workspace = true } [target.'cfg(windows)'.dependencies] windows-sys = { workspace = true, features = [ diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 51c96f436..acd21aa7e 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -10,6 +10,7 @@ mod error; use clap::builder::ValueParser; use clap::{Arg, ArgAction, ArgMatches, Command, error::ErrorKind}; use indicatif::{MultiProgress, ProgressBar, ProgressStyle}; + use std::collections::HashSet; use std::env; use std::ffi::OsString; @@ -17,12 +18,17 @@ use std::fs; use std::io; #[cfg(unix)] use std::os::unix; +#[cfg(unix)] +use std::os::unix::fs::FileTypeExt; #[cfg(windows)] use std::os::windows; use std::path::{Path, PathBuf, absolute}; + use uucore::backup_control::{self, source_is_target_backup}; use uucore::display::Quotable; use uucore::error::{FromIo, UResult, USimpleError, UUsageError, set_exit_code}; +#[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, @@ -665,6 +671,16 @@ fn rename( Ok(()) } +#[cfg(unix)] +fn is_fifo(filetype: fs::FileType) -> bool { + filetype.is_fifo() +} + +#[cfg(not(unix))] +fn is_fifo(_filetype: fs::FileType) -> bool { + false +} + /// A wrapper around `fs::rename`, so that if it fails, we try falling back on /// copying and removing. fn rename_with_fallback( @@ -694,12 +710,28 @@ fn rename_with_fallback( rename_symlink_fallback(from, to) } else if file_type.is_dir() { rename_dir_fallback(from, to, multi_progress) + } else if is_fifo(file_type) { + rename_fifo_fallback(from, to) } else { rename_file_fallback(from, to) } }) } +/// Replace the destination with a new pipe with the same name as the source. +#[cfg(unix)] +fn rename_fifo_fallback(from: &Path, to: &Path) -> io::Result<()> { + if to.try_exists()? { + fs::remove_file(to)?; + } + make_fifo(to).and_then(|_| fs::remove_file(from)) +} + +#[cfg(not(unix))] +fn rename_fifo_fallback(_from: &Path, _to: &Path) -> io::Result<()> { + Ok(()) +} + /// Move the given symlink to the given destination. On Windows, dangling /// symlinks return an error. #[cfg(unix)] diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index d689072a7..577f6a758 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -7,6 +7,8 @@ use filetime::FileTime; use rstest::rstest; use std::io::Write; +#[cfg(not(windows))] +use std::path::Path; use uutests::new_ucmd; use uutests::util::TestScenario; use uutests::{at_and_ucmd, util_name}; @@ -1876,3 +1878,18 @@ fn test_mv_error_msg_with_multiple_sources_that_does_not_exist() { .stderr_contains("mv: cannot stat 'a': No such file or directory") .stderr_contains("mv: cannot stat 'b/': No such file or directory"); } + +#[cfg(not(windows))] +#[ignore = "requires access to a different filesystem"] +#[test] +fn test_special_file_different_filesystem() { + let (at, mut ucmd) = at_and_ucmd!(); + at.mkfifo("f"); + // TODO Use `TestScenario::mount_temp_fs()` for this purpose and + // un-ignore this test. + std::fs::create_dir("/dev/shm/tmp").unwrap(); + ucmd.args(&["f", "/dev/shm/tmp"]).succeeds().no_output(); + assert!(!at.file_exists("f")); + assert!(Path::new("/dev/shm/tmp/f").exists()); + std::fs::remove_dir_all("/dev/shm/tmp").unwrap(); +}