From 0f75117f9d6a33cacc540c858c23d80ad0051391 Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 19 Nov 2025 19:39:04 +0900 Subject: [PATCH 01/22] feat: Add support for long double floating-point numbers and refine general float formatting. --- src/uu/od/src/formatter_item_info.rs | 5 ++ src/uu/od/src/input_decoder.rs | 8 +++ src/uu/od/src/od.rs | 4 ++ src/uu/od/src/output_info.rs | 2 +- src/uu/od/src/parse_formats.rs | 6 +- src/uu/od/src/prn_float.rs | 82 +++++++++++++++++++++++++++- 6 files changed, 103 insertions(+), 4 deletions(-) diff --git a/src/uu/od/src/formatter_item_info.rs b/src/uu/od/src/formatter_item_info.rs index e530a0a3e89..472c9fc4ee2 100644 --- a/src/uu/od/src/formatter_item_info.rs +++ b/src/uu/od/src/formatter_item_info.rs @@ -12,6 +12,7 @@ use std::fmt; pub enum FormatWriter { IntWriter(fn(u64) -> String), FloatWriter(fn(f64) -> String), + LongDoubleWriter(fn(f64) -> String), // On most platforms, long double is f64 or emulated BFloatWriter(fn(f64) -> String), MultibyteWriter(fn(&[u8]) -> String), } @@ -27,6 +28,10 @@ impl fmt::Debug for FormatWriter { f.write_str("FloatWriter:")?; fmt::Pointer::fmt(p, f) } + Self::LongDoubleWriter(ref p) => { + f.write_str("LongDoubleWriter:")?; + fmt::Pointer::fmt(p, f) + } Self::BFloatWriter(ref p) => { f.write_str("BFloatWriter:")?; fmt::Pointer::fmt(p, f) diff --git a/src/uu/od/src/input_decoder.rs b/src/uu/od/src/input_decoder.rs index a65e7613ba5..4045415b021 100644 --- a/src/uu/od/src/input_decoder.rs +++ b/src/uu/od/src/input_decoder.rs @@ -165,6 +165,14 @@ impl MemoryDecoder<'_> { let val = f32::from(bf16::from_bits(bits)); f64::from(val) } + + /// Returns a long double from the internal buffer at position `start`. + /// On most platforms, long double is 64-bit or 80-bit. We read the first 8 bytes as f64. + /// This matches GNU od behavior on platforms where long double == double. + pub fn read_long_double(&self, start: usize) -> f64 { + // Read as f64 (8 bytes) + self.read_float(start, 8) + } } #[cfg(test)] diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 80e6893d1fc..96fa20abb13 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -669,6 +669,10 @@ fn print_bytes(prefix: &str, input_decoder: &MemoryDecoder, output_info: &Output let p = input_decoder.read_float(b, f.formatter_item_info.byte_size); output_text.push_str(&func(p)); } + FormatWriter::LongDoubleWriter(func) => { + let p = input_decoder.read_long_double(b); + output_text.push_str(&func(p)); + } FormatWriter::BFloatWriter(func) => { let p = input_decoder.read_bfloat(b); output_text.push_str(&func(p)); diff --git a/src/uu/od/src/output_info.rs b/src/uu/od/src/output_info.rs index 38218cde8b0..33c3cc70498 100644 --- a/src/uu/od/src/output_info.rs +++ b/src/uu/od/src/output_info.rs @@ -11,7 +11,7 @@ use crate::formatter_item_info::FormatterItemInfo; use crate::parse_formats::ParsedFormatterItemInfo; /// Size in bytes of the max datatype. ie set to 16 for 128-bit numbers. -const MAX_BYTES_PER_UNIT: usize = 8; +const MAX_BYTES_PER_UNIT: usize = 16; /// Contains information to output single output line in human readable form pub struct SpacedFormatterItemInfo { diff --git a/src/uu/od/src/parse_formats.rs b/src/uu/od/src/parse_formats.rs index a62adc1ad33..0edb4847425 100644 --- a/src/uu/od/src/parse_formats.rs +++ b/src/uu/od/src/parse_formats.rs @@ -81,6 +81,7 @@ fn od_format_type(type_char: FormatType, byte_size: u8) -> Option Some(FORMAT_ITEM_F16), (FormatType::Float, 0 | 4) => Some(FORMAT_ITEM_F32), (FormatType::Float, 8) => Some(FORMAT_ITEM_F64), + (FormatType::Float, 16) => Some(FORMAT_ITEM_LONG_DOUBLE), _ => None, } @@ -238,7 +239,10 @@ fn is_format_size_char( *byte_size = 2; true } - // FormatTypeCategory::Float, 'L' => *byte_size = 16, // TODO support f128 + (FormatTypeCategory::Float, Some('L')) => { + *byte_size = 16; + true + } _ => false, } } diff --git a/src/uu/od/src/prn_float.rs b/src/uu/od/src/prn_float.rs index 2e1ff698800..4a3e2cfe3d5 100644 --- a/src/uu/od/src/prn_float.rs +++ b/src/uu/od/src/prn_float.rs @@ -25,6 +25,12 @@ pub static FORMAT_ITEM_F64: FormatterItemInfo = FormatterItemInfo { formatter: FormatWriter::FloatWriter(format_item_f64), }; +pub static FORMAT_ITEM_LONG_DOUBLE: FormatterItemInfo = FormatterItemInfo { + byte_size: 16, + print_width: 40, + formatter: FormatWriter::LongDoubleWriter(format_item_long_double), +}; + pub static FORMAT_ITEM_BF16: FormatterItemInfo = FormatterItemInfo { byte_size: 2, print_width: 16, @@ -43,6 +49,11 @@ pub fn format_item_f64(f: f64) -> String { format!(" {}", format_f64(f)) } +pub fn format_item_long_double(f: f64) -> String { + format!(" {}", format_long_double(f)) +} + + fn format_f32_exp(f: f32, width: usize) -> String { if f.abs().log10() < 0.0 { return format!("{f:width$e}"); @@ -71,11 +82,13 @@ fn format_f64_exp_precision(f: f64, width: usize, precision: usize) -> String { } pub fn format_item_bf16(f: f64) -> String { - format!(" {}", format_f32(f as f32)) + format!(" {}", format_float_simple(f, 15)) } fn format_f16(f: f16) -> String { - format_float(f64::from(f), 15, 8) + // Use a simpler format for f16, similar to %g in C + let val = f64::from(f); + format_float_simple(val, 15) } /// formats float with 8 significant digits, eg 12345678 or -1.2345678e+12 @@ -96,6 +109,41 @@ fn format_f64(f: f64) -> String { format_float(f, 24, 17) } +/// Formats float in a simple way, similar to %g in C +/// Removes trailing zeros and unnecessary decimal points +fn format_float_simple(f: f64, width: usize) -> String { + if !f.is_finite() { + if f.is_nan() { + return format!("{:>width$}", "NaN"); + } else if f.is_sign_negative() { + return format!("{:>width$}", "-inf"); + } else { + return format!("{:>width$}", "inf"); + } + } + + if f == 0.0 { + if f.is_sign_negative() { + return format!("{:>width$}", "-0"); + } else { + return format!("{:>width$}", "0"); + } + } + + // Format with %g style - use exponential for very large/small numbers + let abs_f = f.abs(); + if abs_f < 1e-4 || abs_f >= 1e6 { + // Use exponential notation + let formatted = format!("{:e}", f); + format!("{:>width$}", formatted) + } else { + // Use decimal notation and remove trailing zeros + let formatted = format!("{:.6}", f); + let trimmed = formatted.trim_end_matches('0').trim_end_matches('.'); + format!("{:>width$}", trimmed) + } +} + fn format_float(f: f64, width: usize, precision: usize) -> String { if !f.is_normal() { if f == -0.0 && f.is_sign_negative() { @@ -124,6 +172,36 @@ fn format_float(f: f64, width: usize, precision: usize) -> String { } } +fn format_long_double(f: f64) -> String { + // On most platforms, long double is either 64-bit (same as f64) or 80-bit/128-bit + // Since we're reading it as f64, we format it with extended precision + // Width is 39 (40 - 1 for leading space), precision is 21 significant digits + let width: usize = 39; + let precision: usize = 21; + + // Handle special cases + if f.is_nan() { + return format!("{:>width$}", "NaN"); + } + if f.is_infinite() { + if f.is_sign_negative() { + return format!("{:>width$}", "-inf"); + } else { + return format!("{:>width$}", "inf"); + } + } + if f == 0.0 { + if f.is_sign_negative() { + return format!("{:>width$}", "-0"); + } else { + return format!("{:>width$}", "0"); + } + } + + // For normal numbers, format with appropriate precision using exponential notation + format!("{f:>width$.precision$e}") +} + #[test] #[allow(clippy::excessive_precision)] #[allow(clippy::cognitive_complexity)] From a74b99b6462b8ec037adc46aac6cc1e43b042152 Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 19 Nov 2025 20:29:09 +0900 Subject: [PATCH 02/22] feat: Enhance `od` error reporting for file I/O, width, and offset parsing, including overflow detection and input validation. --- src/uu/od/locales/en-US.ftl | 1 + src/uu/od/src/multifile_reader.rs | 8 ++- src/uu/od/src/od.rs | 33 +++++++++--- src/uu/od/src/parse_inputs.rs | 84 ++++++++++++++++++++++++------- 4 files changed, 99 insertions(+), 27 deletions(-) diff --git a/src/uu/od/locales/en-US.ftl b/src/uu/od/locales/en-US.ftl index 208bd333369..378dfdd18eb 100644 --- a/src/uu/od/locales/en-US.ftl +++ b/src/uu/od/locales/en-US.ftl @@ -55,6 +55,7 @@ od-error-invalid-offset = invalid offset: {$offset} od-error-invalid-label = invalid label: {$label} od-error-too-many-inputs = too many inputs after --traditional: {$input} od-error-parse-failed = parse failed +od-error-overflow = Numerical result out of range od-error-invalid-suffix = invalid suffix in --{$option} argument {$value} od-error-invalid-argument = invalid --{$option} argument {$value} od-error-argument-too-large = --{$option} argument {$value} too large diff --git a/src/uu/od/src/multifile_reader.rs b/src/uu/od/src/multifile_reader.rs index 7d4709ce180..da92a67ea0b 100644 --- a/src/uu/od/src/multifile_reader.rs +++ b/src/uu/od/src/multifile_reader.rs @@ -87,7 +87,13 @@ impl MultifileReader<'_> { // print an error at the time that the file is needed, // then move to the next file. // This matches the behavior of the original `od` - show_error!("{}: {e}", fname.maybe_quote()); + // Format error without OS error code to match GNU od + let error_msg = match e.kind() { + io::ErrorKind::NotFound => "No such file or directory", + io::ErrorKind::PermissionDenied => "Permission denied", + _ => "I/O error", + }; + show_error!("{}: {}", fname.maybe_quote(), error_msg); self.any_err = true; } } diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 96fa20abb13..a34ed0a89c1 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -115,7 +115,7 @@ impl OdOptions { let mut label: Option = None; let parsed_input = parse_inputs(matches) - .map_err(|e| USimpleError::new(1, translate!("od-error-invalid-inputs", "msg" => e)))?; + .map_err(|e| USimpleError::new(1, e))?; let input_strings = match parsed_input { CommandLineInputs::FileNames(v) => v, CommandLineInputs::FileAndOffset((f, s, l)) => { @@ -132,13 +132,32 @@ impl OdOptions { Some(s) => { if matches.value_source(options::WIDTH) == Some(ValueSource::CommandLine) { match parse_number_of_bytes(s) { - Ok(n) => usize::try_from(n) - .map_err(|_| USimpleError::new(1, format!("‘{s}‘ is too large")))?, + Ok(n) => { + let width = usize::try_from(n) + .map_err(|_| USimpleError::new(1, format!("'{s}' is too large")))?; + // Validate width is not zero + if width == 0 { + return Err(USimpleError::new( + 1, + format!("invalid -w argument '{s}'"), + )); + } + width + } Err(e) => { - return Err(USimpleError::new( - 1, - format_error_message(&e, s, options::WIDTH), - )); + // Format error message using -w instead of --width + let error_msg = match e { + ParseSizeError::InvalidSuffix(_) => { + format!("invalid -w argument '{s}'") + } + ParseSizeError::ParseFailure(_) | ParseSizeError::PhysicalMem(_) => { + format!("invalid -w argument '{s}'") + } + ParseSizeError::SizeTooBig(_) => { + format!("invalid -w argument '{s}'") + } + }; + return Err(USimpleError::new(1, error_msg)); } } } else { diff --git a/src/uu/od/src/parse_inputs.rs b/src/uu/od/src/parse_inputs.rs index b185e5427d9..d8a6821d865 100644 --- a/src/uu/od/src/parse_inputs.rs +++ b/src/uu/od/src/parse_inputs.rs @@ -69,17 +69,26 @@ pub fn parse_inputs(matches: &dyn CommandLineOpts) -> Result { + // if there is just 1 input (stdin), an offset must start with '+' + if input_strings.len() == 1 && input_strings[0].starts_with('+') { + return Ok(CommandLineInputs::FileAndOffset(("-".to_string(), n, None))); + } + if input_strings.len() == 2 { + return Ok(CommandLineInputs::FileAndOffset(( + input_strings[0].to_string(), + n, + None, + ))); + } } - if input_strings.len() == 2 { - return Ok(CommandLineInputs::FileAndOffset(( - input_strings[0].to_string(), - n, - None, - ))); + Err(e) => { + // If it's an overflow error, propagate it + // Otherwise, treat it as a filename + if e == translate!("od-error-overflow").leak() { + return Err(format!("{}: {}", input_strings[input_strings.len() - 1], e)); + } } } } @@ -123,7 +132,7 @@ pub fn parse_inputs_traditional(input_strings: &[&str]) -> Result Err(translate!("od-error-invalid-offset", "offset" => input_strings[1])), + (_, Err(e)) => Err(format!("{}: {}", input_strings[1], e)), } } 3 => { @@ -135,12 +144,8 @@ pub fn parse_inputs_traditional(input_strings: &[&str]) -> Result { - Err(translate!("od-error-invalid-offset", "offset" => input_strings[1])) - } - (_, Err(_)) => { - Err(translate!("od-error-invalid-label", "label" => input_strings[2])) - } + (Err(e), _) => Err(format!("{}: {}", input_strings[1], e)), + (_, Err(e)) => Err(format!("{}: {}", input_strings[2], e)), } } _ => Err(translate!("od-error-too-many-inputs", "input" => input_strings[3])), @@ -149,6 +154,26 @@ pub fn parse_inputs_traditional(input_strings: &[&str]) -> Result Result { + // Reject empty strings + if s.is_empty() { + return Err(translate!("od-error-parse-failed").leak()); + } + + // Reject strings with spaces (e.g., "+ 0") + if s.contains(' ') { + return Err(translate!("od-error-parse-failed").leak()); + } + + // Reject strings starting with "++" or "+-" + if s.starts_with("++") || s.starts_with("+-") { + return Err(translate!("od-error-parse-failed").leak()); + } + + // Reject strings starting with "-" (negative numbers not allowed) + if s.starts_with('-') { + return Err(translate!("od-error-parse-failed").leak()); + } + let mut start = 0; let mut len = s.len(); let mut radix = 8; @@ -171,12 +196,33 @@ pub fn parse_offset_operand(s: &str) -> Result { radix = 10; } } + + // Check if the substring is empty after processing prefixes/suffixes + if start >= len { + return Err(translate!("od-error-parse-failed").leak()); + } + match u64::from_str_radix(&s[start..len], radix) { - Ok(i) => Ok(i * multiply), - Err(_) => Err(translate!("od-error-parse-failed").leak()), + Ok(i) => { + // Check for overflow during multiplication + match i.checked_mul(multiply) { + Some(result) => Ok(result), + None => Err(translate!("od-error-overflow").leak()), + } + } + Err(e) => { + // Distinguish between overflow and parse failure + // from_str_radix returns IntErrorKind::PosOverflow for overflow + use std::num::IntErrorKind; + match e.kind() { + IntErrorKind::PosOverflow => Err(translate!("od-error-overflow").leak()), + _ => Err(translate!("od-error-parse-failed").leak()), + } + } } } + #[cfg(test)] mod tests { use super::*; From 2ec9a6aa4b98219233327091d66dbb59c4f75623 Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 19 Nov 2025 21:46:19 +0900 Subject: [PATCH 03/22] feat: Improve long double parsing by converting f128 to f64, enhance overflow error reporting with `libc::ERANGE`, and prevent final offset printing on input errors. --- Cargo.lock | 1 + src/uu/od/Cargo.toml | 1 + src/uu/od/src/byteorder_io.rs | 3 ++- src/uu/od/src/input_decoder.rs | 47 +++++++++++++++++++++++++++++++--- src/uu/od/src/od.rs | 4 ++- src/uu/od/src/parse_inputs.rs | 37 ++++++++++++++++++-------- 6 files changed, 76 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3c8aa58051d..64d86f1e868 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3639,6 +3639,7 @@ dependencies = [ "clap", "fluent", "half", + "libc", "uucore", ] diff --git a/src/uu/od/Cargo.toml b/src/uu/od/Cargo.toml index a2cd59a63e4..97676a86e0a 100644 --- a/src/uu/od/Cargo.toml +++ b/src/uu/od/Cargo.toml @@ -23,6 +23,7 @@ clap = { workspace = true } half = { workspace = true } uucore = { workspace = true, features = ["parser"] } fluent = { workspace = true } +libc.workspace = true [[bin]] name = "od" diff --git a/src/uu/od/src/byteorder_io.rs b/src/uu/od/src/byteorder_io.rs index 545016ff358..8cc7a8bac05 100644 --- a/src/uu/od/src/byteorder_io.rs +++ b/src/uu/od/src/byteorder_io.rs @@ -52,5 +52,6 @@ gen_byte_order_ops! { read_i32, write_i32 -> i32, read_i64, write_i64 -> i64, read_f32, write_f32 -> f32, - read_f64, write_f64 -> f64 + read_f64, write_f64 -> f64, + read_u128, write_u128 -> u128 } diff --git a/src/uu/od/src/input_decoder.rs b/src/uu/od/src/input_decoder.rs index 4045415b021..9d38cf57986 100644 --- a/src/uu/od/src/input_decoder.rs +++ b/src/uu/od/src/input_decoder.rs @@ -167,11 +167,50 @@ impl MemoryDecoder<'_> { } /// Returns a long double from the internal buffer at position `start`. - /// On most platforms, long double is 64-bit or 80-bit. We read the first 8 bytes as f64. - /// This matches GNU od behavior on platforms where long double == double. + /// We read 16 bytes as u128 (respecting endianness) and convert to f64. + /// This ensures that endianness swapping works correctly even if we lose precision. pub fn read_long_double(&self, start: usize) -> f64 { - // Read as f64 (8 bytes) - self.read_float(start, 8) + let bits = self.byte_order.read_u128(&self.data[start..start + 16]); + f128_to_f64(bits) + } +} + +fn f128_to_f64(u: u128) -> f64 { + let sign = (u >> 127) as u64; + let exp = ((u >> 112) & 0x7FFF) as u64; + let mant = (u & ((1 << 112) - 1)) as u128; + + if exp == 0x7FFF { + // Infinity or NaN + if mant == 0 { + if sign == 0 { f64::INFINITY } else { f64::NEG_INFINITY } + } else { + f64::NAN + } + } else if exp == 0 { + // Subnormal or zero + if mant == 0 { + if sign == 0 { 0.0 } else { -0.0 } + } else { + // Subnormal f128 is too small for f64, flush to zero + if sign == 0 { 0.0 } else { -0.0 } + } + } else { + // Normal + let new_exp = exp as i64 - 16383 + 1023; + if new_exp >= 2047 { + // Overflow to infinity + if sign == 0 { f64::INFINITY } else { f64::NEG_INFINITY } + } else if new_exp <= 0 { + // Underflow to zero + if sign == 0 { 0.0 } else { -0.0 } + } else { + // Normal f64 + // Mantissa: take top 52 bits of 112-bit mantissa + let new_mant = (mant >> (112 - 52)) as u64; + let bits = (sign << 63) | ((new_exp as u64) << 52) | new_mant; + f64::from_bits(bits) + } } } diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index a34ed0a89c1..5dad4714d97 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -510,7 +510,9 @@ where let length = memory_decoder.length(); if length == 0 { - input_offset.print_final_offset(); + if !input_decoder.has_error() { + input_offset.print_final_offset(); + } break; } diff --git a/src/uu/od/src/parse_inputs.rs b/src/uu/od/src/parse_inputs.rs index d8a6821d865..45fca948762 100644 --- a/src/uu/od/src/parse_inputs.rs +++ b/src/uu/od/src/parse_inputs.rs @@ -86,7 +86,11 @@ pub fn parse_inputs(matches: &dyn CommandLineOpts) -> Result { // If it's an overflow error, propagate it // Otherwise, treat it as a filename - if e == translate!("od-error-overflow").leak() { + let err = std::io::Error::from_raw_os_error(libc::ERANGE); + let msg = err.to_string(); + let expected_msg = msg.split(" (os error").next().unwrap_or(&msg).to_string(); + + if e == expected_msg { return Err(format!("{}: {}", input_strings[input_strings.len() - 1], e)); } } @@ -153,25 +157,25 @@ pub fn parse_inputs_traditional(input_strings: &[&str]) -> Result Result { +pub fn parse_offset_operand(s: &str) -> Result { // Reject empty strings if s.is_empty() { - return Err(translate!("od-error-parse-failed").leak()); + return Err(translate!("od-error-parse-failed").leak().to_string()); } // Reject strings with spaces (e.g., "+ 0") if s.contains(' ') { - return Err(translate!("od-error-parse-failed").leak()); + return Err(translate!("od-error-parse-failed").leak().to_string()); } // Reject strings starting with "++" or "+-" if s.starts_with("++") || s.starts_with("+-") { - return Err(translate!("od-error-parse-failed").leak()); + return Err(translate!("od-error-parse-failed").leak().to_string()); } // Reject strings starting with "-" (negative numbers not allowed) if s.starts_with('-') { - return Err(translate!("od-error-parse-failed").leak()); + return Err(translate!("od-error-parse-failed").leak().to_string()); } let mut start = 0; @@ -199,7 +203,7 @@ pub fn parse_offset_operand(s: &str) -> Result { // Check if the substring is empty after processing prefixes/suffixes if start >= len { - return Err(translate!("od-error-parse-failed").leak()); + return Err(translate!("od-error-parse-failed").leak().to_string()); } match u64::from_str_radix(&s[start..len], radix) { @@ -207,7 +211,13 @@ pub fn parse_offset_operand(s: &str) -> Result { // Check for overflow during multiplication match i.checked_mul(multiply) { Some(result) => Ok(result), - None => Err(translate!("od-error-overflow").leak()), + None => { + let err = std::io::Error::from_raw_os_error(libc::ERANGE); + let msg = err.to_string(); + // Strip "(os error N)" if present to match Perl's $! + let msg = msg.split(" (os error").next().unwrap_or(&msg).to_string(); + Err(msg) + } } } Err(e) => { @@ -215,8 +225,13 @@ pub fn parse_offset_operand(s: &str) -> Result { // from_str_radix returns IntErrorKind::PosOverflow for overflow use std::num::IntErrorKind; match e.kind() { - IntErrorKind::PosOverflow => Err(translate!("od-error-overflow").leak()), - _ => Err(translate!("od-error-parse-failed").leak()), + IntErrorKind::PosOverflow => { + let err = std::io::Error::from_raw_os_error(libc::ERANGE); + let msg = err.to_string(); + let msg = msg.split(" (os error").next().unwrap_or(&msg).to_string(); + Err(msg) + } + _ => Err(translate!("od-error-parse-failed").leak().to_string()), } } } @@ -386,7 +401,7 @@ mod tests { .unwrap_err(); } - fn parse_offset_operand_str(s: &str) -> Result { + fn parse_offset_operand_str(s: &str) -> Result { parse_offset_operand(&String::from(s)) } From 4de57697ba3568a46c1a8c3517abe6ce99c5083c Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 19 Nov 2025 21:47:59 +0900 Subject: [PATCH 04/22] style: Apply minor formatting adjustments across the `od` module. --- src/uu/od/src/input_decoder.rs | 12 ++++++++++-- src/uu/od/src/od.rs | 6 +++--- src/uu/od/src/parse_inputs.rs | 3 +-- src/uu/od/src/prn_float.rs | 9 ++++----- 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/uu/od/src/input_decoder.rs b/src/uu/od/src/input_decoder.rs index 9d38cf57986..4179e23702a 100644 --- a/src/uu/od/src/input_decoder.rs +++ b/src/uu/od/src/input_decoder.rs @@ -183,7 +183,11 @@ fn f128_to_f64(u: u128) -> f64 { if exp == 0x7FFF { // Infinity or NaN if mant == 0 { - if sign == 0 { f64::INFINITY } else { f64::NEG_INFINITY } + if sign == 0 { + f64::INFINITY + } else { + f64::NEG_INFINITY + } } else { f64::NAN } @@ -200,7 +204,11 @@ fn f128_to_f64(u: u128) -> f64 { let new_exp = exp as i64 - 16383 + 1023; if new_exp >= 2047 { // Overflow to infinity - if sign == 0 { f64::INFINITY } else { f64::NEG_INFINITY } + if sign == 0 { + f64::INFINITY + } else { + f64::NEG_INFINITY + } } else if new_exp <= 0 { // Underflow to zero if sign == 0 { 0.0 } else { -0.0 } diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 5dad4714d97..2ab1b73c233 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -114,8 +114,7 @@ impl OdOptions { let mut label: Option = None; - let parsed_input = parse_inputs(matches) - .map_err(|e| USimpleError::new(1, e))?; + let parsed_input = parse_inputs(matches).map_err(|e| USimpleError::new(1, e))?; let input_strings = match parsed_input { CommandLineInputs::FileNames(v) => v, CommandLineInputs::FileAndOffset((f, s, l)) => { @@ -150,7 +149,8 @@ impl OdOptions { ParseSizeError::InvalidSuffix(_) => { format!("invalid -w argument '{s}'") } - ParseSizeError::ParseFailure(_) | ParseSizeError::PhysicalMem(_) => { + ParseSizeError::ParseFailure(_) + | ParseSizeError::PhysicalMem(_) => { format!("invalid -w argument '{s}'") } ParseSizeError::SizeTooBig(_) => { diff --git a/src/uu/od/src/parse_inputs.rs b/src/uu/od/src/parse_inputs.rs index 45fca948762..ec3021bfcec 100644 --- a/src/uu/od/src/parse_inputs.rs +++ b/src/uu/od/src/parse_inputs.rs @@ -89,7 +89,7 @@ pub fn parse_inputs(matches: &dyn CommandLineOpts) -> Result Result { } } - #[cfg(test)] mod tests { use super::*; diff --git a/src/uu/od/src/prn_float.rs b/src/uu/od/src/prn_float.rs index 4a3e2cfe3d5..585971f493e 100644 --- a/src/uu/od/src/prn_float.rs +++ b/src/uu/od/src/prn_float.rs @@ -53,7 +53,6 @@ pub fn format_item_long_double(f: f64) -> String { format!(" {}", format_long_double(f)) } - fn format_f32_exp(f: f32, width: usize) -> String { if f.abs().log10() < 0.0 { return format!("{f:width$e}"); @@ -121,7 +120,7 @@ fn format_float_simple(f: f64, width: usize) -> String { return format!("{:>width$}", "inf"); } } - + if f == 0.0 { if f.is_sign_negative() { return format!("{:>width$}", "-0"); @@ -129,7 +128,7 @@ fn format_float_simple(f: f64, width: usize) -> String { return format!("{:>width$}", "0"); } } - + // Format with %g style - use exponential for very large/small numbers let abs_f = f.abs(); if abs_f < 1e-4 || abs_f >= 1e6 { @@ -178,7 +177,7 @@ fn format_long_double(f: f64) -> String { // Width is 39 (40 - 1 for leading space), precision is 21 significant digits let width: usize = 39; let precision: usize = 21; - + // Handle special cases if f.is_nan() { return format!("{:>width$}", "NaN"); @@ -197,7 +196,7 @@ fn format_long_double(f: f64) -> String { return format!("{:>width$}", "0"); } } - + // For normal numbers, format with appropriate precision using exponential notation format!("{f:>width$.precision$e}") } From 1f5ec848c57361f5e81e0be4409b909993fcbadd Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 19 Nov 2025 22:06:09 +0900 Subject: [PATCH 05/22] refactor: simplify float formatting logic and update string handling syntax --- src/uu/od/src/input_decoder.rs | 2 +- src/uu/od/src/parse_inputs.rs | 12 ++++++------ src/uu/od/src/prn_float.rs | 22 +++++++++------------- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/src/uu/od/src/input_decoder.rs b/src/uu/od/src/input_decoder.rs index 4179e23702a..986cb2a8df0 100644 --- a/src/uu/od/src/input_decoder.rs +++ b/src/uu/od/src/input_decoder.rs @@ -178,7 +178,7 @@ impl MemoryDecoder<'_> { fn f128_to_f64(u: u128) -> f64 { let sign = (u >> 127) as u64; let exp = ((u >> 112) & 0x7FFF) as u64; - let mant = (u & ((1 << 112) - 1)) as u128; + let mant = u & ((1 << 112) - 1); if exp == 0x7FFF { // Infinity or NaN diff --git a/src/uu/od/src/parse_inputs.rs b/src/uu/od/src/parse_inputs.rs index ec3021bfcec..f9c4f9d88b3 100644 --- a/src/uu/od/src/parse_inputs.rs +++ b/src/uu/od/src/parse_inputs.rs @@ -160,22 +160,22 @@ pub fn parse_inputs_traditional(input_strings: &[&str]) -> Result Result { // Reject empty strings if s.is_empty() { - return Err(translate!("od-error-parse-failed").leak().to_string()); + return Err((*translate!("od-error-parse-failed").leak()).to_string()); } // Reject strings with spaces (e.g., "+ 0") if s.contains(' ') { - return Err(translate!("od-error-parse-failed").leak().to_string()); + return Err((*translate!("od-error-parse-failed").leak()).to_string()); } // Reject strings starting with "++" or "+-" if s.starts_with("++") || s.starts_with("+-") { - return Err(translate!("od-error-parse-failed").leak().to_string()); + return Err((*translate!("od-error-parse-failed").leak()).to_string()); } // Reject strings starting with "-" (negative numbers not allowed) if s.starts_with('-') { - return Err(translate!("od-error-parse-failed").leak().to_string()); + return Err((*translate!("od-error-parse-failed").leak()).to_string()); } let mut start = 0; @@ -203,7 +203,7 @@ pub fn parse_offset_operand(s: &str) -> Result { // Check if the substring is empty after processing prefixes/suffixes if start >= len { - return Err(translate!("od-error-parse-failed").leak().to_string()); + return Err((*translate!("od-error-parse-failed").leak()).to_string()); } match u64::from_str_radix(&s[start..len], radix) { @@ -231,7 +231,7 @@ pub fn parse_offset_operand(s: &str) -> Result { let msg = msg.split(" (os error").next().unwrap_or(&msg).to_string(); Err(msg) } - _ => Err(translate!("od-error-parse-failed").leak().to_string()), + _ => Err((*translate!("od-error-parse-failed").leak()).to_string()), } } } diff --git a/src/uu/od/src/prn_float.rs b/src/uu/od/src/prn_float.rs index 585971f493e..a73cf91eca1 100644 --- a/src/uu/od/src/prn_float.rs +++ b/src/uu/od/src/prn_float.rs @@ -116,30 +116,28 @@ fn format_float_simple(f: f64, width: usize) -> String { return format!("{:>width$}", "NaN"); } else if f.is_sign_negative() { return format!("{:>width$}", "-inf"); - } else { - return format!("{:>width$}", "inf"); } + return format!("{:>width$}", "inf"); } if f == 0.0 { if f.is_sign_negative() { return format!("{:>width$}", "-0"); - } else { - return format!("{:>width$}", "0"); } + return format!("{:>width$}", "0"); } // Format with %g style - use exponential for very large/small numbers let abs_f = f.abs(); - if abs_f < 1e-4 || abs_f >= 1e6 { + if !(1e-4..1e6).contains(&abs_f) { // Use exponential notation - let formatted = format!("{:e}", f); - format!("{:>width$}", formatted) + let formatted = format!("{f:e}"); + format!("{formatted:>width$}") } else { // Use decimal notation and remove trailing zeros - let formatted = format!("{:.6}", f); + let formatted = format!("{f:.6}"); let trimmed = formatted.trim_end_matches('0').trim_end_matches('.'); - format!("{:>width$}", trimmed) + format!("{trimmed:>width$}") } } @@ -185,16 +183,14 @@ fn format_long_double(f: f64) -> String { if f.is_infinite() { if f.is_sign_negative() { return format!("{:>width$}", "-inf"); - } else { - return format!("{:>width$}", "inf"); } + return format!("{:>width$}", "inf"); } if f == 0.0 { if f.is_sign_negative() { return format!("{:>width$}", "-0"); - } else { - return format!("{:>width$}", "0"); } + return format!("{:>width$}", "0"); } // For normal numbers, format with appropriate precision using exponential notation From 69dbb1130a175ddd2718e5de496bf22fd108051c Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 19 Nov 2025 22:10:33 +0900 Subject: [PATCH 06/22] fix: Correct float formatting logic to use decimal for numbers within range and exponential otherwise. --- src/uu/od/src/prn_float.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/uu/od/src/prn_float.rs b/src/uu/od/src/prn_float.rs index a73cf91eca1..2be95807904 100644 --- a/src/uu/od/src/prn_float.rs +++ b/src/uu/od/src/prn_float.rs @@ -129,15 +129,15 @@ fn format_float_simple(f: f64, width: usize) -> String { // Format with %g style - use exponential for very large/small numbers let abs_f = f.abs(); - if !(1e-4..1e6).contains(&abs_f) { - // Use exponential notation - let formatted = format!("{f:e}"); - format!("{formatted:>width$}") - } else { + if (1e-4..1e6).contains(&abs_f) { // Use decimal notation and remove trailing zeros let formatted = format!("{f:.6}"); let trimmed = formatted.trim_end_matches('0').trim_end_matches('.'); format!("{trimmed:>width$}") + } else { + // Use exponential notation + let formatted = format!("{f:e}"); + format!("{formatted:>width$}") } } From 417fd85686bf1e2711d9572e0a21091b3af5d33e Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 19 Nov 2025 22:18:01 +0900 Subject: [PATCH 07/22] refactor(test): use helper function in test_calculate_alignment Replace repetitive assert_eq! calls with a new assert_alignment helper to improve test readability and reduce code duplication. The helper encapsulates alignment checks for OutputInfo::calculate_alignment, making tests clearer and easier to maintain. --- src/uu/od/src/output_info.rs | 390 +++++++++++++++++------------------ 1 file changed, 190 insertions(+), 200 deletions(-) diff --git a/src/uu/od/src/output_info.rs b/src/uu/od/src/output_info.rs index 33c3cc70498..ef63c16026e 100644 --- a/src/uu/od/src/output_info.rs +++ b/src/uu/od/src/output_info.rs @@ -204,6 +204,36 @@ impl TypeSizeInfo for TypeInfo { } } +#[cfg(test)] +fn assert_alignment( + expected: &[usize], + type_info: TypeInfo, + byte_size_block: usize, + print_width_block: usize, +) { + assert_eq!( + expected.len(), + byte_size_block, + "expected spacing must describe every byte in the block" + ); + + let spacing = OutputInfo::calculate_alignment(&type_info, byte_size_block, print_width_block); + + assert_eq!( + expected, + &spacing[..byte_size_block], + "unexpected spacing for byte_size={} print_width={} block_width={}", + type_info.byte_size, + type_info.print_width, + print_width_block + ); + assert!( + spacing[byte_size_block..].iter().all(|&s| s == 0), + "spacing beyond the active block should remain zero: {:?}", + &spacing[byte_size_block..] + ); +} + #[test] #[allow(clippy::cognitive_complexity)] fn test_calculate_alignment() { @@ -213,40 +243,34 @@ fn test_calculate_alignment() { // ffff ffff ffff ffff ffff ffff ffff ffff // the first line has no additional spacing: - assert_eq!( - [0, 0, 0, 0, 0, 0, 0, 0], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 8, - print_width: 23, - }, - 8, - 23 - ) + assert_alignment( + &[0, 0, 0, 0, 0, 0, 0, 0], + TypeInfo { + byte_size: 8, + print_width: 23, + }, + 8, + 23, ); // the second line a single space at the start of the block: - assert_eq!( - [1, 0, 0, 0, 0, 0, 0, 0], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 4, - print_width: 11, - }, - 8, - 23 - ) + assert_alignment( + &[1, 0, 0, 0, 0, 0, 0, 0], + TypeInfo { + byte_size: 4, + print_width: 11, + }, + 8, + 23, ); // the third line two spaces at pos 0, and 1 space at pos 4: - assert_eq!( - [2, 0, 0, 0, 1, 0, 0, 0], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 2, - print_width: 5, - }, - 8, - 23 - ) + assert_alignment( + &[2, 0, 0, 0, 1, 0, 0, 0], + TypeInfo { + byte_size: 2, + print_width: 5, + }, + 8, + 23, ); // For this example `byte_size_block` is 8 and 'print_width_block' is 28: @@ -255,195 +279,161 @@ fn test_calculate_alignment() { // 177777 177777 177777 177777 177777 177777 177777 177777 // ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff - assert_eq!( - [7, 0, 0, 0, 0, 0, 0, 0], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 8, - print_width: 21, - }, - 8, - 28 - ) + assert_alignment( + &[7, 0, 0, 0, 0, 0, 0, 0], + TypeInfo { + byte_size: 8, + print_width: 21, + }, + 8, + 28, ); - assert_eq!( - [5, 0, 0, 0, 5, 0, 0, 0], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 4, - print_width: 9, - }, - 8, - 28 - ) + assert_alignment( + &[5, 0, 0, 0, 5, 0, 0, 0], + TypeInfo { + byte_size: 4, + print_width: 9, + }, + 8, + 28, ); - assert_eq!( - [0, 0, 0, 0, 0, 0, 0, 0], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 2, - print_width: 7, - }, - 8, - 28 - ) + assert_alignment( + &[0, 0, 0, 0, 0, 0, 0, 0], + TypeInfo { + byte_size: 2, + print_width: 7, + }, + 8, + 28, ); - assert_eq!( - [1, 0, 1, 0, 1, 0, 1, 0], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 1, - print_width: 3, - }, - 8, - 28 - ) + assert_alignment( + &[1, 0, 1, 0, 1, 0, 1, 0], + TypeInfo { + byte_size: 1, + print_width: 3, + }, + 8, + 28, ); // 9 tests where 8 .. 16 spaces are spread across 8 positions - assert_eq!( - [1, 1, 1, 1, 1, 1, 1, 1], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 1, - print_width: 2, - }, - 8, - 16 + 8 - ) + assert_alignment( + &[1, 1, 1, 1, 1, 1, 1, 1], + TypeInfo { + byte_size: 1, + print_width: 2, + }, + 8, + 16 + 8, ); - assert_eq!( - [2, 1, 1, 1, 1, 1, 1, 1], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 1, - print_width: 2, - }, - 8, - 16 + 9 - ) + assert_alignment( + &[2, 1, 1, 1, 1, 1, 1, 1], + TypeInfo { + byte_size: 1, + print_width: 2, + }, + 8, + 16 + 9, ); - assert_eq!( - [2, 1, 1, 1, 2, 1, 1, 1], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 1, - print_width: 2, - }, - 8, - 16 + 10 - ) + assert_alignment( + &[2, 1, 1, 1, 2, 1, 1, 1], + TypeInfo { + byte_size: 1, + print_width: 2, + }, + 8, + 16 + 10, ); - assert_eq!( - [3, 1, 1, 1, 2, 1, 1, 1], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 1, - print_width: 2, - }, - 8, - 16 + 11 - ) + assert_alignment( + &[3, 1, 1, 1, 2, 1, 1, 1], + TypeInfo { + byte_size: 1, + print_width: 2, + }, + 8, + 16 + 11, ); - assert_eq!( - [2, 1, 2, 1, 2, 1, 2, 1], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 1, - print_width: 2, - }, - 8, - 16 + 12 - ) + assert_alignment( + &[2, 1, 2, 1, 2, 1, 2, 1], + TypeInfo { + byte_size: 1, + print_width: 2, + }, + 8, + 16 + 12, ); - assert_eq!( - [3, 1, 2, 1, 2, 1, 2, 1], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 1, - print_width: 2, - }, - 8, - 16 + 13 - ) + assert_alignment( + &[3, 1, 2, 1, 2, 1, 2, 1], + TypeInfo { + byte_size: 1, + print_width: 2, + }, + 8, + 16 + 13, ); - assert_eq!( - [3, 1, 2, 1, 3, 1, 2, 1], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 1, - print_width: 2, - }, - 8, - 16 + 14 - ) + assert_alignment( + &[3, 1, 2, 1, 3, 1, 2, 1], + TypeInfo { + byte_size: 1, + print_width: 2, + }, + 8, + 16 + 14, ); - assert_eq!( - [4, 1, 2, 1, 3, 1, 2, 1], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 1, - print_width: 2, - }, - 8, - 16 + 15 - ) + assert_alignment( + &[4, 1, 2, 1, 3, 1, 2, 1], + TypeInfo { + byte_size: 1, + print_width: 2, + }, + 8, + 16 + 15, ); - assert_eq!( - [2, 2, 2, 2, 2, 2, 2, 2], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 1, - print_width: 2, - }, - 8, - 16 + 16 - ) + assert_alignment( + &[2, 2, 2, 2, 2, 2, 2, 2], + TypeInfo { + byte_size: 1, + print_width: 2, + }, + 8, + 16 + 16, ); // 4 tests where 15 spaces are spread across 8, 4, 2 or 1 position(s) - assert_eq!( - [4, 1, 2, 1, 3, 1, 2, 1], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 1, - print_width: 2, - }, - 8, - 16 + 15 - ) + assert_alignment( + &[4, 1, 2, 1, 3, 1, 2, 1], + TypeInfo { + byte_size: 1, + print_width: 2, + }, + 8, + 16 + 15, ); - assert_eq!( - [5, 0, 3, 0, 4, 0, 3, 0], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 2, - print_width: 4, - }, - 8, - 16 + 15 - ) + assert_alignment( + &[5, 0, 3, 0, 4, 0, 3, 0], + TypeInfo { + byte_size: 2, + print_width: 4, + }, + 8, + 16 + 15, ); - assert_eq!( - [8, 0, 0, 0, 7, 0, 0, 0], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 4, - print_width: 8, - }, - 8, - 16 + 15 - ) + assert_alignment( + &[8, 0, 0, 0, 7, 0, 0, 0], + TypeInfo { + byte_size: 4, + print_width: 8, + }, + 8, + 16 + 15, ); - assert_eq!( - [15, 0, 0, 0, 0, 0, 0, 0], - OutputInfo::calculate_alignment( - &TypeInfo { - byte_size: 8, - print_width: 16, - }, - 8, - 16 + 15 - ) + assert_alignment( + &[15, 0, 0, 0, 0, 0, 0, 0], + TypeInfo { + byte_size: 8, + print_width: 16, + }, + 8, + 16 + 15, ); } From de9df127fc28bb265582cd30530fddf820ce57cf Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 19 Nov 2025 22:21:33 +0900 Subject: [PATCH 08/22] feat(cspell): add ERANGE to jargon wordlist Added "ERANGE" to the dictionary to prevent spell checker flagging it as a misspelling, as it's a valid errno constant from C libraries. --- .vscode/cspell.dictionaries/jargon.wordlist.txt | 1 + src/uu/od/src/input_decoder.rs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.vscode/cspell.dictionaries/jargon.wordlist.txt b/.vscode/cspell.dictionaries/jargon.wordlist.txt index a3b51bfedb6..e66d88741eb 100644 --- a/.vscode/cspell.dictionaries/jargon.wordlist.txt +++ b/.vscode/cspell.dictionaries/jargon.wordlist.txt @@ -41,6 +41,7 @@ duplicative dsync endianness enqueue +ERANGE errored executable executables diff --git a/src/uu/od/src/input_decoder.rs b/src/uu/od/src/input_decoder.rs index 986cb2a8df0..f4f9d8e7f23 100644 --- a/src/uu/od/src/input_decoder.rs +++ b/src/uu/od/src/input_decoder.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore bfloat multifile +// spell-checker:ignore bfloat multifile mant use half::{bf16, f16}; use std::io; From b7d34f3bfa6af2e11192b075a4668056febdb073 Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 19 Nov 2025 23:01:27 +0900 Subject: [PATCH 09/22] feat(od): improve width error handling and subnormal float output Refactor width option parsing in OdOptions to use i18n-compatible error messages via translate! macro, consolidating redundant error branches for better maintainability. Enhance float formatting for f16 and bf16 by introducing format_binary16_like helper to properly display subnormal values with exponential notation, removing the obsolete format_float_simple function and adding subnormal detection functions for accurate representation in od's output. --- src/uu/od/src/od.rs | 40 +++++++++--------------- src/uu/od/src/prn_float.rs | 64 ++++++++++++++++---------------------- 2 files changed, 40 insertions(+), 64 deletions(-) diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 2ab1b73c233..5838cccf93e 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -131,33 +131,21 @@ impl OdOptions { Some(s) => { if matches.value_source(options::WIDTH) == Some(ValueSource::CommandLine) { match parse_number_of_bytes(s) { - Ok(n) => { - let width = usize::try_from(n) - .map_err(|_| USimpleError::new(1, format!("'{s}' is too large")))?; - // Validate width is not zero - if width == 0 { - return Err(USimpleError::new( - 1, - format!("invalid -w argument '{s}'"), - )); - } - width - } + Ok(n) => usize::try_from(n).map_err(|_| { + USimpleError::new( + 1, + translate!( + "od-error-argument-too-large", + "option" => options::WIDTH, + "value" => s.quote() + ), + ) + })?, Err(e) => { - // Format error message using -w instead of --width - let error_msg = match e { - ParseSizeError::InvalidSuffix(_) => { - format!("invalid -w argument '{s}'") - } - ParseSizeError::ParseFailure(_) - | ParseSizeError::PhysicalMem(_) => { - format!("invalid -w argument '{s}'") - } - ParseSizeError::SizeTooBig(_) => { - format!("invalid -w argument '{s}'") - } - }; - return Err(USimpleError::new(1, error_msg)); + return Err(USimpleError::new( + 1, + format_error_message(&e, s, options::WIDTH), + )); } } } else { diff --git a/src/uu/od/src/prn_float.rs b/src/uu/od/src/prn_float.rs index 2be95807904..0179df9f40d 100644 --- a/src/uu/od/src/prn_float.rs +++ b/src/uu/od/src/prn_float.rs @@ -2,7 +2,7 @@ // // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -use half::f16; +use half::{bf16, f16}; use std::num::FpCategory; use crate::formatter_item_info::{FormatWriter, FormatterItemInfo}; @@ -81,13 +81,34 @@ fn format_f64_exp_precision(f: f64, width: usize, precision: usize) -> String { } pub fn format_item_bf16(f: f64) -> String { - format!(" {}", format_float_simple(f, 15)) + let value = f as f64; + let bf = bf16::from_f32(value as f32); + format!( + " {}", + format_binary16_like(value, 15, 8, is_subnormal_bf16(bf)) + ) } fn format_f16(f: f16) -> String { - // Use a simpler format for f16, similar to %g in C - let val = f64::from(f); - format_float_simple(val, 15) + let value = f64::from(f); + format_binary16_like(value, 15, 8, is_subnormal_f16(f)) +} + +fn format_binary16_like(value: f64, width: usize, precision: usize, force_exp: bool) -> String { + if force_exp { + return format_f64_exp_precision(value, width, precision - 1); + } + format_float(value, width, precision) +} + +fn is_subnormal_f16(value: f16) -> bool { + let bits = value.to_bits(); + (bits & 0x7C00) == 0 && (bits & 0x03FF) != 0 +} + +fn is_subnormal_bf16(value: bf16) -> bool { + let bits = value.to_bits(); + (bits & 0x7F80) == 0 && (bits & 0x007F) != 0 } /// formats float with 8 significant digits, eg 12345678 or -1.2345678e+12 @@ -108,39 +129,6 @@ fn format_f64(f: f64) -> String { format_float(f, 24, 17) } -/// Formats float in a simple way, similar to %g in C -/// Removes trailing zeros and unnecessary decimal points -fn format_float_simple(f: f64, width: usize) -> String { - if !f.is_finite() { - if f.is_nan() { - return format!("{:>width$}", "NaN"); - } else if f.is_sign_negative() { - return format!("{:>width$}", "-inf"); - } - return format!("{:>width$}", "inf"); - } - - if f == 0.0 { - if f.is_sign_negative() { - return format!("{:>width$}", "-0"); - } - return format!("{:>width$}", "0"); - } - - // Format with %g style - use exponential for very large/small numbers - let abs_f = f.abs(); - if (1e-4..1e6).contains(&abs_f) { - // Use decimal notation and remove trailing zeros - let formatted = format!("{f:.6}"); - let trimmed = formatted.trim_end_matches('0').trim_end_matches('.'); - format!("{trimmed:>width$}") - } else { - // Use exponential notation - let formatted = format!("{f:e}"); - format!("{formatted:>width$}") - } -} - fn format_float(f: f64, width: usize, precision: usize) -> String { if !f.is_normal() { if f == -0.0 && f.is_sign_negative() { From 674b9f070be612737c6898e6ee9d413492b4bafd Mon Sep 17 00:00:00 2001 From: mattsu Date: Wed, 19 Nov 2025 23:30:37 +0900 Subject: [PATCH 10/22] refactor(od): simplify format_item_bf16 by removing redundant variable Remove unnecessary `value` variable in `format_item_bf16` function, eliminating a redundant cast and inline `f` directly for clarity and minor efficiency gain. --- src/uu/od/src/prn_float.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/uu/od/src/prn_float.rs b/src/uu/od/src/prn_float.rs index 0179df9f40d..08db399343d 100644 --- a/src/uu/od/src/prn_float.rs +++ b/src/uu/od/src/prn_float.rs @@ -81,11 +81,10 @@ fn format_f64_exp_precision(f: f64, width: usize, precision: usize) -> String { } pub fn format_item_bf16(f: f64) -> String { - let value = f as f64; - let bf = bf16::from_f32(value as f32); + let bf = bf16::from_f32(f as f32); format!( " {}", - format_binary16_like(value, 15, 8, is_subnormal_bf16(bf)) + format_binary16_like(f, 15, 8, is_subnormal_bf16(bf)) ) } From 55d649e7b3afbec1abe653c32430983478095db2 Mon Sep 17 00:00:00 2001 From: mattsu Date: Thu, 20 Nov 2025 09:03:56 +0900 Subject: [PATCH 11/22] fix(od): standardize option names in error messages Remove hardcoded "--" prefixes from localization strings in en-US.ftl and fr-FR.ftl, replacing with a computed display name that includes "--" and optionally the short form (e.g., "--option" or "--option, -s"). Update parse_bytes_option and read_bytes functions to pass an option_display_name, enabling consistent error message formatting across localizations. Add validation to reject zero width values as invalid arguments. Improves user experience by providing clearer, more consistent option references in error outputs. --- src/uu/od/locales/en-US.ftl | 6 +- src/uu/od/locales/fr-FR.ftl | 6 +- src/uu/od/src/od.rs | 78 ++++++++++--- tests/by-util/test_od.rs | 212 +++++++++++++++++++++++++++++++++--- 4 files changed, 265 insertions(+), 37 deletions(-) diff --git a/src/uu/od/locales/en-US.ftl b/src/uu/od/locales/en-US.ftl index 378dfdd18eb..bcafe1fe2c1 100644 --- a/src/uu/od/locales/en-US.ftl +++ b/src/uu/od/locales/en-US.ftl @@ -56,9 +56,9 @@ od-error-invalid-label = invalid label: {$label} od-error-too-many-inputs = too many inputs after --traditional: {$input} od-error-parse-failed = parse failed od-error-overflow = Numerical result out of range -od-error-invalid-suffix = invalid suffix in --{$option} argument {$value} -od-error-invalid-argument = invalid --{$option} argument {$value} -od-error-argument-too-large = --{$option} argument {$value} too large +od-error-invalid-suffix = invalid suffix in {$option} argument {$value} +od-error-invalid-argument = invalid {$option} argument {$value} +od-error-argument-too-large = {$option} argument {$value} too large od-error-skip-past-end = tried to skip past end of input # Help messages diff --git a/src/uu/od/locales/fr-FR.ftl b/src/uu/od/locales/fr-FR.ftl index cba433b640a..df07eebe61e 100644 --- a/src/uu/od/locales/fr-FR.ftl +++ b/src/uu/od/locales/fr-FR.ftl @@ -56,9 +56,9 @@ od-error-invalid-offset = décalage invalide : {$offset} od-error-invalid-label = étiquette invalide : {$label} od-error-too-many-inputs = trop d'entrées après --traditional : {$input} od-error-parse-failed = échec de l'analyse -od-error-invalid-suffix = suffixe invalide dans l'argument --{$option} {$value} -od-error-invalid-argument = argument --{$option} invalide {$value} -od-error-argument-too-large = argument --{$option} {$value} trop grand +od-error-invalid-suffix = suffixe invalide dans l'argument {$option} {$value} +od-error-invalid-argument = argument {$option} invalide {$value} +od-error-argument-too-large = argument {$option} {$value} trop grand od-error-skip-past-end = tentative d'ignorer au-delà de la fin de l'entrée # Messages d'aide diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 5838cccf93e..80de3088527 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -80,14 +80,19 @@ struct OdOptions { } /// Helper function to parse bytes with error handling -fn parse_bytes_option(matches: &ArgMatches, option_name: &str) -> UResult> { +fn parse_bytes_option( + matches: &ArgMatches, + args: &[String], + option_name: &str, + short: Option, +) -> UResult> { match matches.get_one::(option_name) { None => Ok(None), Some(s) => match parse_number_of_bytes(s) { Ok(n) => Ok(Some(n)), Err(e) => Err(USimpleError::new( 1, - format_error_message(&e, s, option_name), + format_error_message(&e, s, &option_display_name(args, option_name, short)), )), }, } @@ -110,7 +115,8 @@ impl OdOptions { ByteOrder::Native }; - let mut skip_bytes = parse_bytes_option(matches, options::SKIP_BYTES)?.unwrap_or(0); + let mut skip_bytes = + parse_bytes_option(matches, args, options::SKIP_BYTES, Some('j'))?.unwrap_or(0); let mut label: Option = None; @@ -130,24 +136,36 @@ impl OdOptions { None => 16, Some(s) => { if matches.value_source(options::WIDTH) == Some(ValueSource::CommandLine) { - match parse_number_of_bytes(s) { - Ok(n) => usize::try_from(n).map_err(|_| { - USimpleError::new( - 1, - translate!( - "od-error-argument-too-large", - "option" => options::WIDTH, - "value" => s.quote() - ), - ) - })?, + let width_display = option_display_name(args, options::WIDTH, Some('w')); + let parsed = match parse_number_of_bytes(s) { + Ok(n) => n, Err(e) => { return Err(USimpleError::new( 1, - format_error_message(&e, s, options::WIDTH), + format_error_message(&e, s, &width_display), )); } + }; + if parsed == 0 { + return Err(USimpleError::new( + 1, + translate!( + "od-error-invalid-argument", + "option" => width_display.clone(), + "value" => s.quote() + ), + )); } + usize::try_from(parsed).map_err(|_| { + USimpleError::new( + 1, + translate!( + "od-error-argument-too-large", + "option" => width_display.clone(), + "value" => s.quote() + ), + ) + })? } else { 16 } @@ -167,9 +185,9 @@ impl OdOptions { let output_duplicates = matches.get_flag(options::OUTPUT_DUPLICATES); - let read_bytes = parse_bytes_option(matches, options::READ_BYTES)?; + let read_bytes = parse_bytes_option(matches, args, options::READ_BYTES, Some('N'))?; - let string_min_length = match parse_bytes_option(matches, options::STRINGS)? { + let string_min_length = match parse_bytes_option(matches, args, options::STRINGS, Some('S'))? { None => None, Some(n) => Some(usize::try_from(n).map_err(|_| { USimpleError::new( @@ -758,6 +776,32 @@ impl HasError for BufReader { } } +fn option_display_name(args: &[String], option_name: &str, short: Option) -> String { + let long_form = format!("--{option_name}"); + let long_form_with_eq = format!("{long_form}="); + if let Some(short_char) = short { + let short_form = format!("-{short_char}"); + for arg in args.iter().skip(1) { + if !arg.starts_with("--") && arg.starts_with(&short_form) { + return short_form.clone(); + } + } + for arg in args.iter().skip(1) { + if arg == &long_form || arg.starts_with(&long_form_with_eq) { + return long_form.clone(); + } + } + short_form + } else { + for arg in args.iter().skip(1) { + if arg == &long_form || arg.starts_with(&long_form_with_eq) { + return long_form.clone(); + } + } + long_form + } +} + fn format_error_message(error: &ParseSizeError, s: &str, option: &str) -> String { // NOTE: // GNU's od echos affected flag, -N or --read-bytes (-j or --skip-bytes, etc.), depending user's selection diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index d5b7479485f..54be3455178 100644 --- a/tests/by-util/test_od.rs +++ b/tests/by-util/test_od.rs @@ -7,6 +7,8 @@ #[cfg(unix)] use std::io::Read; +#[cfg(target_os = "linux")] +use std::path::Path; use unindent::unindent; use uutests::util::TestScenario; @@ -19,6 +21,27 @@ static ALPHA_OUT: &str = " 0000033 "; +fn erange_message() -> String { + let err = std::io::Error::from_raw_os_error(libc::ERANGE); + let msg = err.to_string(); + msg.split(" (os error").next().unwrap_or(&msg).to_string() +} + +fn run_skip_across_inputs(files: &[(&str, &str)], skip: u64, expected: &str) { + let (at, mut ucmd) = at_and_ucmd!(); + for (name, contents) in files { + at.write(name, contents); + } + + ucmd.arg("-c").arg("-j").arg(skip.to_string()).arg("-An"); + + for (name, _) in files { + ucmd.arg(name); + } + + ucmd.succeeds().stdout_only(expected); +} + #[test] fn test_invalid_arg() { new_ucmd!().arg("--definitely-invalid").fails_with_code(1); @@ -368,22 +391,29 @@ fn test_invalid_width() { #[test] fn test_zero_width() { - let input: [u8; 4] = [0x00, 0x00, 0x00, 0x00]; - let expected_output = unindent( - " - 0000000 000000 - 0000002 000000 - 0000004 - ", - ); - new_ucmd!() .arg("-w0") - .arg("-v") - .run_piped_stdin(&input[..]) - .success() - .stderr_is_bytes("od: warning: invalid width 0; using 2 instead\n".as_bytes()) - .stdout_is(expected_output); + .arg("-An") + .fails_with_code(1) + .stderr_only("od: invalid -w argument '0'\n"); +} + +#[test] +fn test_negative_width_argument() { + new_ucmd!() + .arg("-w-1") + .arg("-An") + .fails_with_code(1) + .stderr_only("od: invalid -w argument '-1'\n"); +} + +#[test] +fn test_non_numeric_width_argument() { + new_ucmd!() + .arg("-ww") + .arg("-An") + .fails_with_code(1) + .stderr_only("od: invalid -w argument 'w'\n"); } #[test] @@ -402,6 +432,42 @@ fn test_width_without_value() { .stdout_only(expected_output); } +#[test] +fn test_very_wide_ascii_output() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("data-a", "x"); + ucmd.arg("-a") + .arg("-w65537") + .arg("-An") + .arg("data-a") + .succeeds() + .stdout_only(" x\n"); +} + +#[test] +fn test_very_wide_char_output() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write("data-c", "x"); + ucmd.arg("-c") + .arg("-w65537") + .arg("-An") + .arg("data-c") + .succeeds() + .stdout_only(" x\n"); +} + +#[test] +fn test_very_wide_hex_byte_output() { + let (at, mut ucmd) = at_and_ucmd!(); + at.write_bytes("data-x", &[0x42]); + ucmd.arg("-tx1") + .arg("-w65537") + .arg("-An") + .arg("data-x") + .succeeds() + .stdout_only(" 42\n"); +} + #[test] fn test_suppress_duplicates() { let input: [u8; 41] = [ @@ -606,6 +672,53 @@ fn test_invalid_offset() { new_ucmd!().arg("-Ab").fails(); } +#[test] +fn test_invalid_traditional_offsets_are_filenames() { + let cases = [("++0", "++0"), ("+-0", "+-0"), ("+ 0", "'+ 0'")]; + + for (input, display) in cases { + new_ucmd!() + .arg(input) + .fails_with_code(1) + .stderr_only(format!("od: {display}: No such file or directory\n")); + } + + new_ucmd!() + .arg("--") + .arg("-0") + .fails_with_code(1) + .stderr_only("od: -0: No such file or directory\n"); +} + +#[test] +fn test_traditional_offset_overflow_diagnosed() { + let erange = erange_message(); + let long_octal = "7".repeat(255); + let long_decimal = format!("{}.", "9".repeat(254)); + let long_hex = format!("0x{}", "f".repeat(253)); + + new_ucmd!() + .arg("-") + .arg(&long_octal) + .pipe_in(Vec::::new()) + .fails_with_code(1) + .stderr_only(format!("od: {long_octal}: {erange}\n")); + + new_ucmd!() + .arg("-") + .arg(&long_decimal) + .pipe_in(Vec::::new()) + .fails_with_code(1) + .stderr_only(format!("od: {long_decimal}: {erange}\n")); + + new_ucmd!() + .arg("-") + .arg(&long_hex) + .pipe_in(Vec::::new()) + .fails_with_code(1) + .stderr_only(format!("od: {long_hex}: {erange}\n")); +} + #[test] fn test_empty_offset() { new_ucmd!() @@ -670,6 +783,59 @@ fn test_skip_bytes_hex() { )); } +#[test] +fn test_skip_bytes_consumes_single_input() { + run_skip_across_inputs(&[("g", "a")], 1, ""); +} + +#[test] +fn test_skip_bytes_consumes_two_inputs() { + run_skip_across_inputs(&[("g", "a"), ("h", "b")], 2, ""); +} + +#[test] +fn test_skip_bytes_consumes_three_inputs() { + run_skip_across_inputs(&[("g", "a"), ("h", "b"), ("i", "c")], 3, ""); +} + +#[test] +fn test_skip_bytes_prints_after_consuming_multiple_inputs() { + run_skip_across_inputs( + &[("g", "a"), ("h", "b"), ("i", "c"), ("j", "d")], + 3, + " d\n", + ); +} + +#[cfg(target_os = "linux")] +#[test] +fn test_skip_bytes_proc_file_without_seeking() { + let proc_path = Path::new("/proc/version"); + if !proc_path.exists() { + return; + } + + let Ok(contents) = std::fs::read(proc_path) else { + return; + }; + + if contents.is_empty() { + return; + } + + let (at, mut ucmd) = at_and_ucmd!(); + at.write("after", "e"); + + ucmd.arg("-An") + .arg("-c") + .arg("-j") + .arg(contents.len().to_string()) + .arg(proc_path) + .arg("after") + .succeeds() + .stdout_only(" e\n"); +} + #[test] fn test_skip_bytes_error() { let input = "12345"; @@ -778,6 +944,24 @@ fn test_stdin_offset() { )); } +#[test] +fn test_traditional_decimal_dot_offset() { + new_ucmd!() + .arg("+1.") + .pipe_in("a") + .succeeds() + .stdout_only("0000001\n"); +} + +#[test] +fn test_traditional_dot_block_offset() { + new_ucmd!() + .arg("+1.b") + .pipe_in(vec![b'a'; 512]) + .succeeds() + .stdout_only("0001000\n"); +} + #[test] fn test_file_offset() { new_ucmd!() From ea5417b47850a09ef1a2a60e05c876e47cb5f03b Mon Sep 17 00:00:00 2001 From: mattsu Date: Thu, 20 Nov 2025 09:06:40 +0900 Subject: [PATCH 12/22] refactor: condense format! macro in format_item_bf16 for readability Removed unnecessary line breaks in the format! expression, keeping the code more concise while maintaining functionality. This improves code style in the float printing module. --- src/uu/od/src/prn_float.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/uu/od/src/prn_float.rs b/src/uu/od/src/prn_float.rs index 08db399343d..155ce7d0794 100644 --- a/src/uu/od/src/prn_float.rs +++ b/src/uu/od/src/prn_float.rs @@ -82,10 +82,7 @@ fn format_f64_exp_precision(f: f64, width: usize, precision: usize) -> String { pub fn format_item_bf16(f: f64) -> String { let bf = bf16::from_f32(f as f32); - format!( - " {}", - format_binary16_like(f, 15, 8, is_subnormal_bf16(bf)) - ) + format!(" {}", format_binary16_like(f, 15, 8, is_subnormal_bf16(bf))) } fn format_f16(f: f16) -> String { From 8b83715e2130f395c6e950e9e11597efecbaf3ea Mon Sep 17 00:00:00 2001 From: mattsu Date: Thu, 20 Nov 2025 09:52:46 +0900 Subject: [PATCH 13/22] fix(od): add external quoting for filenames in error messages The MultifileReader now uses `fname.maybe_quote().external(true)` when displaying permission and I/O errors, ensuring filenames are properly quoted for user-facing output (e.g., handling special characters that might confuse shells). This prevents potential issues with filename display in error logs. --- src/uu/od/src/multifile_reader.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/od/src/multifile_reader.rs b/src/uu/od/src/multifile_reader.rs index da92a67ea0b..48e1f1225db 100644 --- a/src/uu/od/src/multifile_reader.rs +++ b/src/uu/od/src/multifile_reader.rs @@ -93,7 +93,7 @@ impl MultifileReader<'_> { io::ErrorKind::PermissionDenied => "Permission denied", _ => "I/O error", }; - show_error!("{}: {}", fname.maybe_quote(), error_msg); + show_error!("{}: {}", fname.maybe_quote().external(true), error_msg); self.any_err = true; } } From 409d0a8eba88762b1388f4eb75645ea80866f021 Mon Sep 17 00:00:00 2001 From: mattsu Date: Thu, 27 Nov 2025 19:16:17 +0900 Subject: [PATCH 14/22] refactor(od): Rename f128_to_f64 to u128_to_f64 for clarity Renamed the function in input_decoder.rs from f128_to_f64 to u128_to_f64 to accurately reflect its purpose of converting u128 integer bits to f64, improving code readability and reducing potential confusion over float types. --- src/uu/od/src/input_decoder.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/od/src/input_decoder.rs b/src/uu/od/src/input_decoder.rs index f4f9d8e7f23..416badb444b 100644 --- a/src/uu/od/src/input_decoder.rs +++ b/src/uu/od/src/input_decoder.rs @@ -171,11 +171,11 @@ impl MemoryDecoder<'_> { /// This ensures that endianness swapping works correctly even if we lose precision. pub fn read_long_double(&self, start: usize) -> f64 { let bits = self.byte_order.read_u128(&self.data[start..start + 16]); - f128_to_f64(bits) + u128_to_f64(bits) } } -fn f128_to_f64(u: u128) -> f64 { +fn u128_to_f64(u: u128) -> f64 { let sign = (u >> 127) as u64; let exp = ((u >> 112) & 0x7FFF) as u64; let mant = u & ((1 << 112) - 1); From 59fe994a263324ea57bbf76e13f8bdc0d5f9e0f2 Mon Sep 17 00:00:00 2001 From: mattsu Date: Thu, 27 Nov 2025 19:19:31 +0900 Subject: [PATCH 15/22] refactor(od): simplify error handling in OdOptions using combinators Use map_err and the try operator to replace a verbose match statement, making the code more concise and idiomatic Rust. This improves readability without altering functionality. --- src/uu/od/src/od.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 80de3088527..11cbd6c6dc5 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -137,15 +137,9 @@ impl OdOptions { Some(s) => { if matches.value_source(options::WIDTH) == Some(ValueSource::CommandLine) { let width_display = option_display_name(args, options::WIDTH, Some('w')); - let parsed = match parse_number_of_bytes(s) { - Ok(n) => n, - Err(e) => { - return Err(USimpleError::new( - 1, - format_error_message(&e, s, &width_display), - )); - } - }; + let parsed = parse_number_of_bytes(s).map_err(|e| { + USimpleError::new(1, format_error_message(&e, s, &width_display)) + })?; if parsed == 0 { return Err(USimpleError::new( 1, From 457abea6f9025a0aa1816c4f873b53829815b9bd Mon Sep 17 00:00:00 2001 From: mattsu <35655889+mattsu2020@users.noreply.github.com> Date: Fri, 28 Nov 2025 08:33:22 +0900 Subject: [PATCH 16/22] Update src/uu/od/src/od.rs Co-authored-by: Daniel Hofstetter --- src/uu/od/src/od.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 11cbd6c6dc5..257e0111d7f 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -777,7 +777,7 @@ fn option_display_name(args: &[String], option_name: &str, short: Option) let short_form = format!("-{short_char}"); for arg in args.iter().skip(1) { if !arg.starts_with("--") && arg.starts_with(&short_form) { - return short_form.clone(); + return short_form; } } for arg in args.iter().skip(1) { From af5cc17a72844f2b25d4bd4f838a30ba103dd8e5 Mon Sep 17 00:00:00 2001 From: mattsu <35655889+mattsu2020@users.noreply.github.com> Date: Fri, 28 Nov 2025 08:33:35 +0900 Subject: [PATCH 17/22] Update src/uu/od/src/od.rs Co-authored-by: Daniel Hofstetter --- src/uu/od/src/od.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 257e0111d7f..2c053652ff7 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -782,7 +782,7 @@ fn option_display_name(args: &[String], option_name: &str, short: Option) } for arg in args.iter().skip(1) { if arg == &long_form || arg.starts_with(&long_form_with_eq) { - return long_form.clone(); + return long_form; } } short_form From a49a88e80bddab3d223aba54e20b9b72a9a5feba Mon Sep 17 00:00:00 2001 From: mattsu <35655889+mattsu2020@users.noreply.github.com> Date: Fri, 28 Nov 2025 08:34:35 +0900 Subject: [PATCH 18/22] Update src/uu/od/src/parse_inputs.rs Co-authored-by: Daniel Hofstetter --- src/uu/od/src/parse_inputs.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/uu/od/src/parse_inputs.rs b/src/uu/od/src/parse_inputs.rs index f9c4f9d88b3..1fbcd3bfe7d 100644 --- a/src/uu/od/src/parse_inputs.rs +++ b/src/uu/od/src/parse_inputs.rs @@ -158,7 +158,6 @@ pub fn parse_inputs_traditional(input_strings: &[&str]) -> Result Result { - // Reject empty strings if s.is_empty() { return Err((*translate!("od-error-parse-failed").leak()).to_string()); } From 6056548120f8743340e7f931602fb52fd11800f5 Mon Sep 17 00:00:00 2001 From: mattsu <35655889+mattsu2020@users.noreply.github.com> Date: Fri, 28 Nov 2025 08:36:38 +0900 Subject: [PATCH 19/22] Update src/uu/od/src/od.rs Co-authored-by: Daniel Hofstetter --- src/uu/od/src/od.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/uu/od/src/od.rs b/src/uu/od/src/od.rs index 2c053652ff7..e8f9841b162 100644 --- a/src/uu/od/src/od.rs +++ b/src/uu/od/src/od.rs @@ -787,11 +787,6 @@ fn option_display_name(args: &[String], option_name: &str, short: Option) } short_form } else { - for arg in args.iter().skip(1) { - if arg == &long_form || arg.starts_with(&long_form_with_eq) { - return long_form.clone(); - } - } long_form } } From 28e3baab1e8ef6486249507056ad920fc3522ed5 Mon Sep 17 00:00:00 2001 From: mattsu <35655889+mattsu2020@users.noreply.github.com> Date: Fri, 28 Nov 2025 08:36:50 +0900 Subject: [PATCH 20/22] Update src/uu/od/src/parse_inputs.rs Co-authored-by: Daniel Hofstetter --- src/uu/od/src/parse_inputs.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/uu/od/src/parse_inputs.rs b/src/uu/od/src/parse_inputs.rs index 1fbcd3bfe7d..c19b655166d 100644 --- a/src/uu/od/src/parse_inputs.rs +++ b/src/uu/od/src/parse_inputs.rs @@ -162,7 +162,6 @@ pub fn parse_offset_operand(s: &str) -> Result { return Err((*translate!("od-error-parse-failed").leak()).to_string()); } - // Reject strings with spaces (e.g., "+ 0") if s.contains(' ') { return Err((*translate!("od-error-parse-failed").leak()).to_string()); } From 827d1ae868d2a7e3ddc1f4024068bd635e7d4105 Mon Sep 17 00:00:00 2001 From: mattsu <35655889+mattsu2020@users.noreply.github.com> Date: Fri, 28 Nov 2025 08:37:08 +0900 Subject: [PATCH 21/22] Update src/uu/od/src/parse_inputs.rs Co-authored-by: Daniel Hofstetter --- src/uu/od/src/parse_inputs.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/uu/od/src/parse_inputs.rs b/src/uu/od/src/parse_inputs.rs index c19b655166d..31eaf9a98f2 100644 --- a/src/uu/od/src/parse_inputs.rs +++ b/src/uu/od/src/parse_inputs.rs @@ -166,7 +166,6 @@ pub fn parse_offset_operand(s: &str) -> Result { return Err((*translate!("od-error-parse-failed").leak()).to_string()); } - // Reject strings starting with "++" or "+-" if s.starts_with("++") || s.starts_with("+-") { return Err((*translate!("od-error-parse-failed").leak()).to_string()); } From 68946bdbad340201298b3642a4e153d2d7a7cf83 Mon Sep 17 00:00:00 2001 From: mattsu Date: Fri, 28 Nov 2025 08:41:15 +0900 Subject: [PATCH 22/22] refactor(od): remove leaking from translated error messages in parse_offset_operand Eliminated use of `.leak()` and unnecessary `.to_string()` calls on translated error strings in the `parse_offset_operand` function. This simplifies error handling, improves memory safety by avoiding intentional leaks, and makes the code cleaner without functional changes. --- src/uu/od/src/parse_inputs.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/uu/od/src/parse_inputs.rs b/src/uu/od/src/parse_inputs.rs index 31eaf9a98f2..8f5e6434b65 100644 --- a/src/uu/od/src/parse_inputs.rs +++ b/src/uu/od/src/parse_inputs.rs @@ -159,20 +159,20 @@ pub fn parse_inputs_traditional(input_strings: &[&str]) -> Result Result { if s.is_empty() { - return Err((*translate!("od-error-parse-failed").leak()).to_string()); + return Err(translate!("od-error-parse-failed")); } if s.contains(' ') { - return Err((*translate!("od-error-parse-failed").leak()).to_string()); + return Err(translate!("od-error-parse-failed")); } if s.starts_with("++") || s.starts_with("+-") { - return Err((*translate!("od-error-parse-failed").leak()).to_string()); + return Err(translate!("od-error-parse-failed")); } // Reject strings starting with "-" (negative numbers not allowed) if s.starts_with('-') { - return Err((*translate!("od-error-parse-failed").leak()).to_string()); + return Err(translate!("od-error-parse-failed")); } let mut start = 0; @@ -200,7 +200,7 @@ pub fn parse_offset_operand(s: &str) -> Result { // Check if the substring is empty after processing prefixes/suffixes if start >= len { - return Err((*translate!("od-error-parse-failed").leak()).to_string()); + return Err(translate!("od-error-parse-failed")); } match u64::from_str_radix(&s[start..len], radix) { @@ -228,7 +228,7 @@ pub fn parse_offset_operand(s: &str) -> Result { let msg = msg.split(" (os error").next().unwrap_or(&msg).to_string(); Err(msg) } - _ => Err((*translate!("od-error-parse-failed").leak()).to_string()), + _ => Err(translate!("od-error-parse-failed")), } } }