diff --git a/src/cmp.rs b/src/cmp.rs index 0f7250c..609fe25 100644 --- a/src/cmp.rs +++ b/src/cmp.rs @@ -12,7 +12,7 @@ use std::process::ExitCode; use std::{cmp, fs, io}; #[cfg(unix)] -use std::os::fd::{AsRawFd, FromRawFd}; +use std::os::fd::{AsFd, AsRawFd, FromRawFd}; #[cfg(unix)] use std::os::unix::fs::MetadataExt; @@ -311,9 +311,36 @@ pub enum Cmp { Different, } +fn is_directory(path: &OsString, params: &Params) -> Result { + if path != "-" { + return Ok(fs::metadata(path).is_ok_and(|metadata| metadata.is_dir())); + } + #[cfg(unix)] + { + io::stdin() + .as_fd() + .try_clone_to_owned() + .and_then(|fd| fs::File::from(fd).metadata()) + .map(|metadata| metadata.is_dir()) + .map_err(|e| format_failure_to_read_input_file(¶ms.executable, path, &e)) + } + #[cfg(not(unix))] + { + let _ = params; + Ok(false) + } +} + pub fn cmp(params: &Params) -> Result { + // With -n 0 a directory operand must still be read, so that it is reported. + let reads_directory = match params.max_bytes { + Some(0) => is_directory(¶ms.from, params) + .and_then(|is_dir| Ok(is_dir || is_directory(¶ms.to, params)?)), + _ => Ok(false), + }; let mut from = prepare_reader(¶ms.from, ¶ms.skip_a, params)?; let mut to = prepare_reader(¶ms.to, ¶ms.skip_b, params)?; + let stop_at_limit = !reads_directory?; let mut offset_width = params.max_bytes.unwrap_or(BytesLimitU64::MAX); @@ -326,8 +353,21 @@ pub fn cmp(params: &Params) -> Result { // If the files have different sizes, we already know they are not identical. If we have not // been asked to show even the first difference, we can quit early. - if params.quiet && a_size != b_size { - return Ok(Cmp::Different); + if params.quiet + && params.from != "-" + && params.to != "-" + && a_meta.is_file() + && b_meta.is_file() + { + let remaining = |size: u64, skip: Option| { + let remaining = size.saturating_sub(skip.unwrap_or(0)); + params + .max_bytes + .map_or(remaining, |max_bytes| remaining.min(max_bytes)) + }; + if remaining(a_size, params.skip_a) != remaining(b_size, params.skip_b) { + return Ok(Cmp::Different); + } } let smaller = cmp::min(a_size, b_size) as BytesLimitU64; @@ -345,6 +385,14 @@ pub fn cmp(params: &Params) -> Result { let mut stdout = BufWriter::new(io::stdout().lock()); let mut compare = Cmp::Equal; loop { + if stop_at_limit + && params + .max_bytes + .is_some_and(|max_bytes| at_byte > max_bytes) + { + break; + } + // Fill up our buffers. let from_buf = from .fill_buf() @@ -373,6 +421,9 @@ pub fn cmp(params: &Params) -> Result { // Fast path - for long files in which almost all bytes are the same we // can do a direct comparison to let the compiler optimize. let consumed = std::cmp::min(from_buf.len(), to_buf.len()); + let consumed = params.max_bytes.map_or(consumed, |max_bytes| { + consumed.min(usize::try_from(max_bytes - (at_byte - 1)).unwrap_or(usize::MAX)) + }); if from_buf[..consumed] == to_buf[..consumed] { let last = from_buf[..consumed].last().unwrap(); @@ -381,12 +432,6 @@ pub fn cmp(params: &Params) -> Result { start_of_line = *last == b'\n'; - if let Some(max_bytes) = params.max_bytes { - if at_byte > max_bytes { - break; - } - } - from.consume(consumed); to.consume(consumed); @@ -395,7 +440,7 @@ pub fn cmp(params: &Params) -> Result { // Iterate over the buffers, the zip iterator will stop us as soon as the // first one runs out. - for (&from_byte, &to_byte) in from_buf.iter().zip(to_buf.iter()) { + for (&from_byte, &to_byte) in from_buf[..consumed].iter().zip(&to_buf[..consumed]) { if from_byte != to_byte { compare = Cmp::Different; @@ -427,12 +472,6 @@ pub fn cmp(params: &Params) -> Result { } at_byte += 1; - - if let Some(max_bytes) = params.max_bytes { - if at_byte > max_bytes { - break; - } - } } // Notify our readers about the bytes we went over. diff --git a/tests/integration.rs b/tests/integration.rs index 12aabb8..a28a534 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -3,12 +3,14 @@ // For the full copyright and license information, please view the LICENSE-* // files that was distributed with this source code. +use assert_cmd::assert::OutputAssertExt; use assert_cmd::cargo::cargo_bin_cmd; use predicates::prelude::*; use std::fs::File; #[cfg(not(windows))] use std::fs::OpenOptions; use std::io::Write; +use std::process::Stdio; use tempfile::{tempdir, NamedTempFile}; // Integration tests for the diffutils command @@ -642,6 +644,79 @@ mod cmp { Ok(()) } + #[track_caller] + fn assert_cmp(dir: &std::path::Path, args: &[&str], stdin: Stdio, code: i32, stderr: &str) { + std::process::Command::new(assert_cmd::cargo::cargo_bin!("diffutils")) + .env("LC_ALL", "C") + .current_dir(dir) + .arg("cmp") + .args(args) + .stdin(stdin) + .output() + .unwrap() + .assert() + .append_context("args", format!("{args:?}")) + .code(predicate::eq(code)) + .stdout(predicate::str::is_empty()) + .stderr(predicate::eq(stderr)); + } + + #[test] + fn cmp_quiet_bytes_limit() -> Result<(), Box> { + let tmp_dir = tempdir()?; + let dir = tmp_dir.path(); + for name in ["a", "ab", "b", "abc", "xabc"] { + std::fs::write(dir.join(name), name)?; + } + for (args, code) in [ + (&["-s", "-n", "1", "a", "ab"][..], 0), + (&["-s", "-n", "2", "a", "ab"][..], 1), + (&["-s", "-n", "0", "a", "b"][..], 0), + (&["-s", "-i", "1:0", "xabc", "abc"][..], 0), + (&["-s", "-i", "1:0", "-n", "2", "xabc", "ab"][..], 0), + ] { + assert_cmp(dir, args, Stdio::null(), code, ""); + } + std::fs::write(dir.join("-"), "zz")?; + for (args, stdin) in [ + (&["-s", "-", "abc"][..], "abc"), + (&["-s", "-i", "0:1", "abc", "-"], "xabc"), + ] { + assert_cmp(dir, args, File::open(dir.join(stdin))?.into(), 0, ""); + } + Ok(()) + } + + #[test] + #[cfg(unix)] + fn cmp_directory_and_device_operands() -> Result<(), Box> { + let tmp_dir = tempdir()?; + let dir = tmp_dir.path(); + std::fs::write(dir.join("file"), "a")?; + std::fs::write(dir.join("nul"), "\0")?; + std::fs::create_dir(dir.join("dir"))?; + for (args, code, stderr) in [ + (&["-s", "file", "dir"][..], 2, ""), + ( + &["-n", "0", "file", "dir"][..], + 2, + "cmp: dir: Is a directory\n", + ), + (&["-s", "-n", "1", "/dev/zero", "nul"][..], 0, ""), + ] { + assert_cmp(dir, args, Stdio::null(), code, stderr); + } + let stdin = File::open(dir)?.into(); + assert_cmp( + dir, + &["-n", "0", "file", "-"], + stdin, + 2, + "cmp: -: Is a directory\n", + ); + Ok(()) + } + #[test] fn cmp_skip_args_parsing() -> Result<(), Box> { let tmp_dir = tempdir()?; @@ -829,7 +904,8 @@ mod cmp { // validating the /dev/null optimization. let a_path = tmp_dir.path().join("a"); let a = File::create(&a_path).unwrap(); - a.set_len(14 * 1024 * 1024 * 1024 * 1024).unwrap(); + let a_len = 14 * 1024 * 1024 * 1024 * 1024; + a.set_len(a_len).unwrap(); let b_path = tmp_dir.path().join("b"); let b = File::create(&b_path).unwrap(); @@ -837,31 +913,35 @@ mod cmp { let dev_null = OpenOptions::new().write(true).open("/dev/null").unwrap(); - let mut child = std::process::Command::new(assert_cmd::cargo::cargo_bin!("diffutils")) - .arg("cmp") - .arg(&a_path) - .arg(&b_path) - .stdout(dev_null) - .spawn() - .unwrap(); - - // Bound the runtime to a very short time that still allows for some resource - // constraint to slow it down while also allowing very fast systems to exit as - // early as possible. - const MAX_TRIES: u8 = 50; - for tries in 0..=MAX_TRIES { - if tries == MAX_TRIES { - panic!("cmp took too long to run, /dev/null optimization probably not working") - } - match child.try_wait() { - Ok(Some(status)) => { - assert_eq!(status.code(), Some(1)); - break; + let limit = (a_len + 1).to_string(); + for args in [&[][..], &["-i", "0"], &["-i", "1"], &["-n", &limit]] { + let mut child = std::process::Command::new(assert_cmd::cargo::cargo_bin!("diffutils")) + .arg("cmp") + .args(args) + .arg(&a_path) + .arg(&b_path) + .stdout(dev_null.try_clone().unwrap()) + .spawn() + .unwrap(); + + // Bound the runtime to a very short time that still allows for some resource + // constraint to slow it down while also allowing very fast systems to exit as + // early as possible. + const MAX_TRIES: u8 = 50; + for tries in 0..=MAX_TRIES { + if tries == MAX_TRIES { + panic!("cmp {args:?} took too long to run, /dev/null optimization probably not working") + } + match child.try_wait() { + Ok(Some(status)) => { + assert_eq!(status.code(), Some(1)); + break; + } + Ok(None) => (), + Err(e) => panic!("{e:#?}"), } - Ok(None) => (), - Err(e) => panic!("{e:#?}"), + std::thread::sleep(std::time::Duration::from_millis(10)); } - std::thread::sleep(std::time::Duration::from_millis(10)); } // Two stdins should be equal