diff --git a/src/cmp.rs b/src/cmp.rs index d1121ce..18a6a07 100644 --- a/src/cmp.rs +++ b/src/cmp.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE-* // files that was distributed with this source code. -use crate::utils::{format_failure_to_read_input_file, is_posix_locale}; +use crate::utils::{format_failure_to_read_input_file, is_posix_locale, is_same_file}; use std::env::ArgsOs; use std::ffi::OsString; use std::io::{BufRead, BufReader, BufWriter, Read, Write}; @@ -450,9 +450,7 @@ pub fn main(opts: Peekable) -> ExitCode { } }; - if params.from == "-" && params.to == "-" - || same_file::is_same_file(¶ms.from, ¶ms.to).unwrap_or(false) - { + if params.from == "-" && params.to == "-" || is_same_file(¶ms.from, ¶ms.to) { return ExitCode::SUCCESS; } diff --git a/src/diff.rs b/src/diff.rs index 7de5f55..e9861cb 100644 --- a/src/diff.rs +++ b/src/diff.rs @@ -5,7 +5,7 @@ use crate::color::{colorize, ColorWhen, Palette, PaletteError}; use crate::params::{parse_params, Format, MissingFile, Params}; -use crate::utils::report_failure_to_read_input_file; +use crate::utils::{is_same_file, report_failure_to_read_input_file}; use crate::{context_diff, dir_diff, ed_diff, normal_diff, side_diff, unified_diff}; use std::env::ArgsOs; use std::ffi::OsString; @@ -80,9 +80,7 @@ pub fn compare_files(params: &Params, header: Option<&[u8]>) -> Outcome { } }; // if from and to are the same file, no need to perform any comparison - if params.from == "-" && params.to == "-" - || same_file::is_same_file(¶ms.from, ¶ms.to).unwrap_or(false) - { + if params.from == "-" && params.to == "-" || is_same_file(¶ms.from, ¶ms.to) { maybe_report_identical_files(); return Outcome::Identical; } diff --git a/src/utils.rs b/src/utils.rs index 29ab93d..d834e89 100644 --- a/src/utils.rs +++ b/src/utils.rs @@ -163,6 +163,26 @@ pub fn quote_shell_argument(arg: &[u8]) -> Cow<'_, [u8]> { Cow::Owned(quoted) } +/// Returns whether both paths lead to the same file. +/// +/// On Unix this compares what stat() reports and never opens the files: +/// opening a FIFO just to identify it would take its writer away from the +/// read that follows. +pub fn is_same_file(from: &OsStr, to: &OsStr) -> bool { + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + match (std::fs::metadata(from), std::fs::metadata(to)) { + (Ok(from), Ok(to)) => from.dev() == to.dev() && from.ino() == to.ino(), + _ => false, + } + } + #[cfg(not(unix))] + { + same_file::is_same_file(from, to).unwrap_or(false) + } +} + /// Returns `name` in lowercase, to match file names ignoring case. pub fn fold_file_name_case(name: &OsStr) -> Vec { match name.to_str() { diff --git a/tests/by-util/test_cmp.rs b/tests/by-util/test_cmp.rs index d7ad3a1..8c79f1c 100644 --- a/tests/by-util/test_cmp.rs +++ b/tests/by-util/test_cmp.rs @@ -61,6 +61,30 @@ fn cmp_stdin(#[case] input: &str, #[case] code: i32, #[case] expected: &str) { .stdout_only(expected); } +// Checking whether the operands are the same file must not open them: a FIFO +// can only be read once. +#[cfg(unix)] +#[test] +fn cmp_fifo_operands_are_read_once() { + let (at, ts) = at_and_ts!(); + let mut writers = Vec::new(); + for (name, content) in [("one", "a\nb\n"), ("two", "a\nc\n")] { + at.mkfifo(name); + let fifo = at.plus(name); + writers.push(std::thread::spawn(move || std::fs::write(fifo, content))); + } + + ts.ucmd() + .args(&["-s", "one", "two"]) + .timeout(std::time::Duration::from_secs(10)) + .run() + .code_is(1) + .no_output(); + for writer in writers { + writer.join().unwrap().unwrap(); + } +} + #[test] fn cmp_equal_files() { let (at, ts) = at_and_ts!(); diff --git a/tests/by-util/test_diff.rs b/tests/by-util/test_diff.rs index c6d4140..a3ba48d 100644 --- a/tests/by-util/test_diff.rs +++ b/tests/by-util/test_diff.rs @@ -574,6 +574,30 @@ fn recursive_loop_on_one_side_only() { .stderr_is("diff: one/sub/loop/sub/loop: recursive directory loop\n"); } +// A FIFO can only be read once: checking whether the operands are the same +// file must not open them, or the writer's data is gone before the read. +#[cfg(unix)] +#[test] +fn fifo_operands_are_read_once() { + let (at, ts) = at_and_ts!(); + let mut writers = Vec::new(); + for (name, content) in [("one", "a\nb\n"), ("two", "a\nc\n")] { + at.mkfifo(name); + let fifo = at.plus(name); + writers.push(std::thread::spawn(move || std::fs::write(fifo, content))); + } + + ts.ucmd() + .args(&["one", "two"]) + .timeout(std::time::Duration::from_secs(10)) + .run() + .code_is(1) + .stdout_only("2c2\n< b\n---\n> c\n"); + for writer in writers { + writer.join().unwrap().unwrap(); + } +} + #[test] fn directory_name_matching() { let (at, ts) = at_and_ts!();