diff --git a/src/cmp.rs b/src/cmp.rs index 3ff716f..df58269 100644 --- a/src/cmp.rs +++ b/src/cmp.rs @@ -317,8 +317,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; @@ -336,6 +349,15 @@ pub fn cmp(params: &Params) -> Result { let mut stdout = BufWriter::new(io::stdout().lock()); let mut compare = Cmp::Equal; loop { + // Once the limit is reached, don't wait for more input. + if at_byte > 1 + && params + .max_bytes + .is_some_and(|max_bytes| at_byte > max_bytes) + { + break; + } + // Fill up our buffers. let from_buf = from .fill_buf() @@ -345,6 +367,13 @@ pub fn cmp(params: &Params) -> Result { .fill_buf() .map_err(|e| format_failure_to_read_input_file(¶ms.executable, ¶ms.to, &e))?; + if params + .max_bytes + .is_some_and(|max_bytes| at_byte > max_bytes) + { + break; + } + // Check for EOF conditions. if from_buf.is_empty() && to_buf.is_empty() { break; @@ -364,6 +393,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(); @@ -372,12 +404,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); @@ -386,7 +412,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; @@ -418,12 +444,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/by-util/test_cmp.rs b/tests/by-util/test_cmp.rs index 0053a46..26447b4 100644 --- a/tests/by-util/test_cmp.rs +++ b/tests/by-util/test_cmp.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}; // A file whose metadata length is 0 but which still yields bytes (/dev/zero) @@ -318,6 +320,108 @@ fn cmp_max_bytes() -> Result<(), Box> { 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] +#[cfg(unix)] +fn cmp_bytes_limit_with_open_pipe() -> Result<(), Box> { + let tmp_dir = tempdir()?; + let file = tmp_dir.path().join("file"); + std::fs::write(&file, "abcde")?; + + let mut child = std::process::Command::new(assert_cmd::cargo::cargo_bin!("diffutils")) + .args(["cmp", "-n", "5", "-"]) + .arg(&file) + .stdin(Stdio::piped()) + .stdout(Stdio::null()) + .spawn()?; + // The write end stays open, so another read would block. + let mut stdin = child.stdin.take().unwrap(); + stdin.write_all(b"abcde")?; + + for _ in 0..500 { + if let Some(status) = child.try_wait()? { + assert_eq!(status.code(), Some(0)); + return Ok(()); + } + std::thread::sleep(std::time::Duration::from_millis(10)); + } + child.kill()?; + child.wait()?; + panic!("cmp -n 5 kept reading after the limit"); +} + #[test] fn cmp_skip_args_parsing() -> Result<(), Box> { let tmp_dir = tempdir()?; @@ -505,7 +609,8 @@ fn cmp_fast_paths() -> Result<(), Box> { // 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(); @@ -513,31 +618,35 @@ fn cmp_fast_paths() -> Result<(), Box> { 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") } - Ok(None) => (), - Err(e) => panic!("{e:#?}"), + match child.try_wait() { + Ok(Some(status)) => { + assert_eq!(status.code(), Some(1)); + break; + } + 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