From e736b44706a41030b1c0930293ef5be0c9db6ef6 Mon Sep 17 00:00:00 2001 From: Jay Hemnani <193022578+jayhemnani9910@users.noreply.github.com> Date: Tue, 20 Oct 2026 04:09:33 +0530 Subject: [PATCH 1/2] cmp: accept unit suffixes in --bytes -n/--bytes only took a plain number, so "cmp -n 1K a b" failed with "invalid --bytes value", while GNU cmp takes the same suffixes there as for --ignore-initial. Share the --ignore-initial parser, with the option name in its error message, and turn the test that expected 1K to fail into cases for -n 1K, --bytes=1kB and --bytes=2M. --- src/cmp.rs | 49 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 32 insertions(+), 17 deletions(-) diff --git a/src/cmp.rs b/src/cmp.rs index 0f7250c0..ab9d9b8b 100644 --- a/src/cmp.rs +++ b/src/cmp.rs @@ -74,7 +74,8 @@ pub fn parse_params>(mut opts: Peekable) -> Resu let executable = opts.next().ok_or("Usage: ".to_string())?; let executable_str = executable.to_string_lossy().to_string(); - let parse_skip = |param: &str, skip_desc: &str| -> Result { + // Sizes for --ignore-initial and --bytes, with the same unit suffixes. + let parse_size = |param: &str, option: &str, desc: &str| -> Result { let suffix_start = param .find(|b: char| !b.is_ascii_digit()) .unwrap_or(param.len()); @@ -83,7 +84,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu Err(e) if *e.kind() == std::num::IntErrorKind::PosOverflow => SkipU64::MAX, Err(_) => { return Err(format!( - "{executable_str}: invalid --ignore-initial value '{skip_desc}'" + "{executable_str}: invalid --{option} value '{desc}'" )) } }; @@ -111,7 +112,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu "Y" => SkipU64::MAX, // 1_208_925_819_614_629_174_706_176, _ => { return Err(format!( - "{executable_str}: invalid --ignore-initial value '{skip_desc}'" + "{executable_str}: invalid --{option} value '{desc}'" )); } }; @@ -124,6 +125,9 @@ pub fn parse_params>(mut opts: Peekable) -> Resu Ok(num) }; + let parse_skip = |param: &str, skip_desc: &str| -> Result { + parse_size(param, "ignore-initial", skip_desc) + }; let mut params = Params { executable, @@ -172,17 +176,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu let (_, arg) = param_str.split_once('=').unwrap(); arg.to_string() }; - let max_bytes = match max_bytes.parse::() { - Ok(num) => num, - // TODO limit to MAX is dangerous, this should become an error like in GNU cmp. - Err(e) if *e.kind() == std::num::IntErrorKind::PosOverflow => BytesLimitU64::MAX, - Err(_) => { - return Err(format!( - "{executable_str}: invalid --bytes value '{max_bytes}'" - )) - } - }; - params.max_bytes = Some(max_bytes); + params.max_bytes = Some(parse_size(&max_bytes, "bytes", &max_bytes)?); continue; } if param == "-i" || param_str.starts_with("--ignore-initial=") { @@ -985,12 +979,33 @@ mod tests { ) ); + // Same unit suffixes as --ignore-initial + for (args, bytes) in [ + (&["-n", "1K"][..], 1_024), + (&["--bytes=1kB"][..], 1_000), + (&["--bytes=2M"][..], 2_097_152), + ] { + let mut opts = vec![os("cmp")]; + opts.extend(args.iter().map(|arg| os(arg))); + opts.extend([os("foo"), os("bar")]); + assert_eq!( + Ok(Params { + executable: os("cmp"), + from: os("foo"), + to: os("bar"), + max_bytes: Some(bytes), + ..Default::default() + }), + parse_params(opts.into_iter().peekable()), + "{args:?}" + ); + } + // Failure case - // TODO This is actually fine in GNU cmp. --bytes does not have a unit parser yet. assert_eq!( - Err("cmp: invalid --bytes value '1K'".to_string()), + Err("cmp: invalid --bytes value '1x'".to_string()), parse_params( - [os("cmp"), os("--bytes=1K"), os("foo"), os("bar")] + [os("cmp"), os("--bytes=1x"), os("foo"), os("bar")] .iter() .cloned() .peekable() From f89e83b5150ed8721eefb0d60a176a4b0159d4ef Mon Sep 17 00:00:00 2001 From: Jay Hemnani <193022578+jayhemnani9910@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:06:31 +0530 Subject: [PATCH 2/2] cmp: call parse_size directly and test -n 1K --- src/cmp.rs | 17 +++++++---------- tests/integration.rs | 13 +++++++++++++ 2 files changed, 20 insertions(+), 10 deletions(-) diff --git a/src/cmp.rs b/src/cmp.rs index ab9d9b8b..3ff716f0 100644 --- a/src/cmp.rs +++ b/src/cmp.rs @@ -125,9 +125,6 @@ pub fn parse_params>(mut opts: Peekable) -> Resu Ok(num) }; - let parse_skip = |param: &str, skip_desc: &str| -> Result { - parse_size(param, "ignore-initial", skip_desc) - }; let mut params = Params { executable, @@ -191,11 +188,11 @@ pub fn parse_params>(mut opts: Peekable) -> Resu }; let (skip_a, skip_b) = if let Some((skip_a, skip_b)) = skip_desc.split_once(':') { ( - parse_skip(skip_a, &skip_desc)?, - parse_skip(skip_b, &skip_desc)?, + parse_size(skip_a, "ignore-initial", &skip_desc)?, + parse_size(skip_b, "ignore-initial", &skip_desc)?, ) } else { - let skip = parse_skip(&skip_desc, &skip_desc)?; + let skip = parse_size(&skip_desc, "ignore-initial", &skip_desc)?; (skip, skip) }; params.skip_a = Some(skip_a); @@ -218,9 +215,9 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } else if to.is_none() { to = Some(param); } else if skip_pos1.is_none() { - skip_pos1 = Some(parse_skip(¶m_str, ¶m_str)?); + skip_pos1 = Some(parse_size(¶m_str, "ignore-initial", ¶m_str)?); } else if skip_pos2.is_none() { - skip_pos2 = Some(parse_skip(¶m_str, ¶m_str)?); + skip_pos2 = Some(parse_size(¶m_str, "ignore-initial", ¶m_str)?); } else { return Err(usage_string(&executable_str)); } @@ -262,7 +259,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu params.skip_a = skip_pos1; } else if let Some(param) = opts.next() { let param_str = param.to_string_lossy().to_string(); - params.skip_a = Some(parse_skip(¶m_str, ¶m_str)?); + params.skip_a = Some(parse_size(¶m_str, "ignore-initial", ¶m_str)?); } }; if params.skip_b.is_none() { @@ -270,7 +267,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu params.skip_b = skip_pos2; } else if let Some(param) = opts.next() { let param_str = param.to_string_lossy().to_string(); - params.skip_b = Some(parse_skip(¶m_str, ¶m_str)?); + params.skip_b = Some(parse_size(¶m_str, "ignore-initial", ¶m_str)?); } } diff --git a/tests/integration.rs b/tests/integration.rs index ad5efd92..28fd8ad9 100644 --- a/tests/integration.rs +++ b/tests/integration.rs @@ -669,6 +669,19 @@ mod cmp { cmd.arg("-n"); cmd.arg("13"); cmd.arg(&a_path).arg(&b_path); + cmd.assert() + .code(predicate::eq(1)) + .failure() + .stderr(predicate::str::is_empty()) + .stdout(predicate::eq(" 4 40 144 d\n 8 40 150 h\n")); + + let mut cmd = cargo_bin_cmd!("diffutils"); + cmd.arg("cmp"); + cmd.arg("-l"); + cmd.arg("-b"); + cmd.arg("-n"); + cmd.arg("1K"); + cmd.arg(&a_path).arg(&b_path); cmd.assert() .code(predicate::eq(1)) .failure()