diff, cmp: check for the same file without opening the operands

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<ArgsOs>) -> ExitCode {
         }
     };
 
-    if params.from == "-" && params.to == "-"
-        || same_file::is_same_file(&params.from, &params.to).unwrap_or(false)
-    {
+    if params.from == "-" && params.to == "-" || is_same_file(&params.from, &params.to) {
         return ExitCode::SUCCESS;
     }
 

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(&params.from, &params.to).unwrap_or(false)
-    {
+    if params.from == "-" && params.to == "-" || is_same_file(&params.from, &params.to) {
         maybe_report_identical_files();
         return Outcome::Identical;
     }

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<u8> {
     match name.to_str() {

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!();

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!();