From f105031dfaa307bd0fa32e52db3668ee6fbdfdad Mon Sep 17 00:00:00 2001 From: qezem Date: Sat, 14 Nov 2026 10:44:24 +0500 Subject: [PATCH 1/3] cmp: match GNU usage errors messages --- src/cmp.rs | 81 ++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 61 insertions(+), 20 deletions(-) diff --git a/src/cmp.rs b/src/cmp.rs index d1121ce6..d03df40e 100644 --- a/src/cmp.rs +++ b/src/cmp.rs @@ -38,6 +38,10 @@ pub struct Params { quiet: bool, } +fn usage_error(executable: &str, msg: &str) -> String { + format!("{executable}: {msg}\n{executable}: Try '{executable} --help' for more information.") +} + #[inline] fn usage_string(executable: &str) -> String { format!("Usage: {executable} ") @@ -83,9 +87,10 @@ pub fn parse_params>(mut opts: Peekable) -> Resu Ok(num) => num, Err(e) if *e.kind() == std::num::IntErrorKind::PosOverflow => SkipU64::MAX, Err(_) => { - return Err(format!( - "{executable_str}: invalid --{option} value '{desc}'" - )) + return Err(usage_error( + &executable_str, + &format!("invalid --{option} value '{desc}'"), + )); } }; @@ -111,8 +116,9 @@ pub fn parse_params>(mut opts: Peekable) -> Resu "YB" => SkipU64::MAX, // 1_000_000_000_000_000_000_000_000, "Y" => SkipU64::MAX, // 1_208_925_819_614_629_174_706_176, _ => { - return Err(format!( - "{executable_str}: invalid --{option} value '{desc}'" + return Err(usage_error( + &executable_str, + &format!("invalid --{option} value '{desc}'"), )); } }; @@ -134,7 +140,12 @@ pub fn parse_params>(mut opts: Peekable) -> Resu let mut to = None; let mut skip_pos1 = None; let mut skip_pos2 = None; + + let mut last_arg = executable_str.clone(); + while let Some(param) = opts.next() { + last_arg = param.to_string_lossy().to_string(); + if param == "--" { break; } @@ -144,7 +155,10 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } else if to.is_none() { to = Some(param); } else { - return Err(usage_string(&executable_str)); + return Err(usage_error( + &executable_str, + "invalid --ignore-initial value '-'", + )); } continue; } @@ -166,7 +180,9 @@ pub fn parse_params>(mut opts: Peekable) -> Resu if param == "-n" || param_str.starts_with("--bytes=") { let max_bytes = if param == "-n" { opts.next() - .ok_or_else(|| usage_string(&executable_str))? + .ok_or_else(|| { + usage_error(&executable_str, "option requires an argument -- 'n'") + })? .to_string_lossy() .to_string() } else { @@ -179,7 +195,9 @@ pub fn parse_params>(mut opts: Peekable) -> Resu if param == "-i" || param_str.starts_with("--ignore-initial=") { let skip_desc = if param == "-i" { opts.next() - .ok_or_else(|| usage_string(&executable_str))? + .ok_or_else(|| { + usage_error(&executable_str, "option requires an argument -- 'i'") + })? .to_string_lossy() .to_string() } else { @@ -208,7 +226,10 @@ pub fn parse_params>(mut opts: Peekable) -> Resu std::process::exit(0); } if param_str.starts_with('-') { - return Err(format!("unrecognized option '{}'", param.to_string_lossy())); + return Err(usage_error( + &executable_str, + &format!("unrecognized option '{}'", param.to_string_lossy()), + )); } if from.is_none() { from = Some(param); @@ -219,7 +240,10 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } else if skip_pos2.is_none() { skip_pos2 = Some(parse_size(¶m_str, "ignore-initial", ¶m_str)?); } else { - return Err(usage_string(&executable_str)); + return Err(usage_error( + &executable_str, + &format!("extra operand '{param_str}'"), + )); } } @@ -233,8 +257,9 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } if params.quiet && params.verbose { - return Err(format!( - "{executable_str}: options -l and -s are incompatible" + return Err(usage_error( + &executable_str, + "options -l and -s are incompatible", )); } @@ -243,7 +268,10 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } else if let Some(param) = opts.next() { param } else { - return Err(usage_string(&executable_str)); + return Err(usage_error( + &executable_str, + &format!("missing operand after '{last_arg}'"), + )); }; params.to = if let Some(to) = to { to @@ -266,6 +294,13 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } } + if let Some(extra) = opts.next() { + return Err(usage_error( + &executable_str, + &format!("extra operand '{}'", extra.to_string_lossy()), + )); + } + // Like GNU cmp, use the larger of the -i value and the positional skip. params.skip_a = params.skip_a.max(skip_pos1); params.skip_b = params.skip_b.max(skip_pos2); @@ -782,7 +817,7 @@ mod tests { // Bad positional arguments. assert_eq!( - Err("Usage: cmp ".to_string()), + Err("cmp: extra operand '3'\ncmp: Try 'cmp --help' for more information.".to_string()), parse_params( [os("cmp"), os("foo"), os("bar"), os("1"), os("2"), os("3")] .iter() @@ -791,7 +826,10 @@ mod tests { ) ); assert_eq!( - Err("Usage: cmp ".to_string()), + Err( + "cmp: missing operand after 'cmp'\ncmp: Try 'cmp --help' for more information." + .to_string() + ), parse_params([os("cmp")].iter().cloned().peekable()) ); } @@ -904,7 +942,7 @@ mod tests { // Some options do not mix. assert_eq!( - Err("cmp: options -l and -s are incompatible".to_string()), + Err("cmp: options -l and -s are incompatible\ncmp: Try 'cmp --help' for more information.".to_string()), parse_params( [os("cmp"), os("-l"), os("-s"), os("foo"), os("bar")] .iter() @@ -987,7 +1025,10 @@ mod tests { // Failure case assert_eq!( - Err("cmp: invalid --bytes value '1x'".to_string()), + Err( + "cmp: invalid --bytes value '1x'\ncmp: Try 'cmp --help' for more information." + .to_string() + ), parse_params( [os("cmp"), os("--bytes=1x"), os("foo"), os("bar")] .iter() @@ -1216,7 +1257,7 @@ mod tests { // Failure cases assert_eq!( - Err("cmp: invalid --ignore-initial value '1mb'".to_string()), + Err("cmp: invalid --ignore-initial value '1mb'\ncmp: Try 'cmp --help' for more information.".to_string()), parse_params( [os("cmp"), os("--ignore-initial=1mb"), os("foo"), os("bar")] .iter() @@ -1225,7 +1266,7 @@ mod tests { ) ); assert_eq!( - Err("cmp: invalid --ignore-initial value '1:2:3'".to_string()), + Err("cmp: invalid --ignore-initial value '1:2:3'\ncmp: Try 'cmp --help' for more information.".to_string()), parse_params( [ os("cmp"), @@ -1239,7 +1280,7 @@ mod tests { ) ); assert_eq!( - Err("cmp: invalid --ignore-initial value '-1'".to_string()), + Err("cmp: invalid --ignore-initial value '-1'\ncmp: Try 'cmp --help' for more information.".to_string()), parse_params( [os("cmp"), os("--ignore-initial=-1"), os("foo"), os("bar")] .iter() From 8f700f9a650008d29fcc6e4ff90af6734ef37491 Mon Sep 17 00:00:00 2001 From: qezem Date: Sat, 14 Nov 2026 11:28:10 +0500 Subject: [PATCH 2/3] cmp: update incompatible params test and optimize last_arg --- src/cmp.rs | 6 +++--- tests/by-util/test_cmp.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cmp.rs b/src/cmp.rs index d03df40e..ba7a5a30 100644 --- a/src/cmp.rs +++ b/src/cmp.rs @@ -141,10 +141,10 @@ pub fn parse_params>(mut opts: Peekable) -> Resu let mut skip_pos1 = None; let mut skip_pos2 = None; - let mut last_arg = executable_str.clone(); + let mut last_arg = params.executable.clone(); while let Some(param) = opts.next() { - last_arg = param.to_string_lossy().to_string(); + last_arg = param.clone(); if param == "--" { break; @@ -270,7 +270,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } else { return Err(usage_error( &executable_str, - &format!("missing operand after '{last_arg}'"), + &format!("missing operand after '{}'", last_arg.to_string_lossy()), )); }; params.to = if let Some(to) = to { diff --git a/tests/by-util/test_cmp.rs b/tests/by-util/test_cmp.rs index d7ad3a1f..2043e4ec 100644 --- a/tests/by-util/test_cmp.rs +++ b/tests/by-util/test_cmp.rs @@ -43,7 +43,7 @@ fn cmp_incompatible_params() { new_ucmd!() .args(&["-l", "-s", "/etc/passwd", "/etc/group"]) .fails_with_code(2) - .stderr_str_check(|s| s.ends_with(": options -l and -s are incompatible\n")); + .stderr_str_check(|s| s.contains(": options -l and -s are incompatible\n")); } #[rstest] From 6c210fda89e0d473480b0ae4e0110ffc3b42fe07 Mon Sep 17 00:00:00 2001 From: qezem Date: Sat, 14 Nov 2026 12:18:10 +0500 Subject: [PATCH 3/3] cmp: optimize parse_params --- src/cmp.rs | 58 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 32 insertions(+), 26 deletions(-) diff --git a/src/cmp.rs b/src/cmp.rs index ba7a5a30..2b5441f1 100644 --- a/src/cmp.rs +++ b/src/cmp.rs @@ -80,9 +80,12 @@ pub fn parse_params>(mut opts: Peekable) -> Resu // 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()); + let bytes = param.as_bytes(); + let mut suffix_start = 0; + + while suffix_start < bytes.len() && bytes[suffix_start].is_ascii_digit() { + suffix_start += 1; + } let mut num = match param[..suffix_start].parse::() { Ok(num) => num, Err(e) if *e.kind() == std::num::IntErrorKind::PosOverflow => SkipU64::MAX, @@ -123,15 +126,24 @@ pub fn parse_params>(mut opts: Peekable) -> Resu } }; - num = match num.overflowing_mul(multiplier) { - (n, false) => n, - _ => SkipU64::MAX, - } + num = num.saturating_mul(multiplier); } Ok(num) }; + let parse_skip = |skip_desc: &str| -> Result<(SkipU64, SkipU64), String> { + if let Some((skip_a, skip_b)) = skip_desc.split_once(':') { + Ok(( + parse_size(skip_a, "ignore-initial", skip_desc)?, + parse_size(skip_b, "ignore-initial", skip_desc)?, + )) + } else { + let skip = parse_size(skip_desc, "ignore-initial", skip_desc)?; + Ok((skip, skip)) + } + }; + let mut params = Params { executable, ..Default::default() @@ -144,7 +156,9 @@ pub fn parse_params>(mut opts: Peekable) -> Resu let mut last_arg = params.executable.clone(); while let Some(param) = opts.next() { - last_arg = param.clone(); + if to.is_none() { + last_arg = param.clone(); + } if param == "--" { break; @@ -176,7 +190,7 @@ pub fn parse_params>(mut opts: Peekable) -> Resu continue; } - let param_str = param.to_string_lossy().to_string(); + let param_str = param.to_string_lossy(); if param == "-n" || param_str.starts_with("--bytes=") { let max_bytes = if param == "-n" { opts.next() @@ -193,26 +207,18 @@ pub fn parse_params>(mut opts: Peekable) -> Resu continue; } if param == "-i" || param_str.starts_with("--ignore-initial=") { - let skip_desc = if param == "-i" { - opts.next() - .ok_or_else(|| { - usage_error(&executable_str, "option requires an argument -- 'i'") - })? - .to_string_lossy() - .to_string() + let (skip_a, skip_b) = if param == "-i" { + let value = opts.next().ok_or_else(|| { + usage_error(&executable_str, "option requires an argument -- 'i'") + })?; + + let skip_desc = value.to_string_lossy(); + parse_skip(&skip_desc)? } else { let (_, arg) = param_str.split_once('=').unwrap(); - arg.to_string() - }; - let (skip_a, skip_b) = if let Some((skip_a, skip_b)) = skip_desc.split_once(':') { - ( - parse_size(skip_a, "ignore-initial", &skip_desc)?, - parse_size(skip_b, "ignore-initial", &skip_desc)?, - ) - } else { - let skip = parse_size(&skip_desc, "ignore-initial", &skip_desc)?; - (skip, skip) + parse_skip(arg)? }; + params.skip_a = params.skip_a.max(Some(skip_a)); params.skip_b = params.skip_b.max(Some(skip_b)); continue;