Skip to content

Commit 43dd238

Browse files
od: make GNU test od.pl pass (#9334)
* feat: Add support for long double floating-point numbers and refine general float formatting. * feat: Enhance `od` error reporting for file I/O, width, and offset parsing, including overflow detection and input validation. * 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. * style: Apply minor formatting adjustments across the `od` module. * refactor: simplify float formatting logic and update string handling syntax * fix: Correct float formatting logic to use decimal for numbers within range and exponential otherwise. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * 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. * Update src/uu/od/src/od.rs Co-authored-by: Daniel Hofstetter <[email protected]> * Update src/uu/od/src/od.rs Co-authored-by: Daniel Hofstetter <[email protected]> * Update src/uu/od/src/parse_inputs.rs Co-authored-by: Daniel Hofstetter <[email protected]> * Update src/uu/od/src/od.rs Co-authored-by: Daniel Hofstetter <[email protected]> * Update src/uu/od/src/parse_inputs.rs Co-authored-by: Daniel Hofstetter <[email protected]> * Update src/uu/od/src/parse_inputs.rs Co-authored-by: Daniel Hofstetter <[email protected]> * 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. --------- Co-authored-by: Daniel Hofstetter <[email protected]>
1 parent f6d581f commit 43dd238

File tree

15 files changed

+675
-266
lines changed

15 files changed

+675
-266
lines changed

.vscode/cspell.dictionaries/jargon.wordlist.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ duplicative
4141
dsync
4242
endianness
4343
enqueue
44+
ERANGE
4445
errored
4546
executable
4647
executables

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/uu/od/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ clap = { workspace = true }
2323
half = { workspace = true }
2424
uucore = { workspace = true, features = ["parser"] }
2525
fluent = { workspace = true }
26+
libc.workspace = true
2627

2728
[[bin]]
2829
name = "od"

src/uu/od/locales/en-US.ftl

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,9 +55,10 @@ od-error-invalid-offset = invalid offset: {$offset}
5555
od-error-invalid-label = invalid label: {$label}
5656
od-error-too-many-inputs = too many inputs after --traditional: {$input}
5757
od-error-parse-failed = parse failed
58-
od-error-invalid-suffix = invalid suffix in --{$option} argument {$value}
59-
od-error-invalid-argument = invalid --{$option} argument {$value}
60-
od-error-argument-too-large = --{$option} argument {$value} too large
58+
od-error-overflow = Numerical result out of range
59+
od-error-invalid-suffix = invalid suffix in {$option} argument {$value}
60+
od-error-invalid-argument = invalid {$option} argument {$value}
61+
od-error-argument-too-large = {$option} argument {$value} too large
6162
od-error-skip-past-end = tried to skip past end of input
6263
6364
# Help messages

src/uu/od/locales/fr-FR.ftl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,9 @@ od-error-invalid-offset = décalage invalide : {$offset}
5656
od-error-invalid-label = étiquette invalide : {$label}
5757
od-error-too-many-inputs = trop d'entrées après --traditional : {$input}
5858
od-error-parse-failed = échec de l'analyse
59-
od-error-invalid-suffix = suffixe invalide dans l'argument --{$option} {$value}
60-
od-error-invalid-argument = argument --{$option} invalide {$value}
61-
od-error-argument-too-large = argument --{$option} {$value} trop grand
59+
od-error-invalid-suffix = suffixe invalide dans l'argument {$option} {$value}
60+
od-error-invalid-argument = argument {$option} invalide {$value}
61+
od-error-argument-too-large = argument {$option} {$value} trop grand
6262
od-error-skip-past-end = tentative d'ignorer au-delà de la fin de l'entrée
6363
6464
# Messages d'aide

src/uu/od/src/byteorder_io.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,5 +52,6 @@ gen_byte_order_ops! {
5252
read_i32, write_i32 -> i32,
5353
read_i64, write_i64 -> i64,
5454
read_f32, write_f32 -> f32,
55-
read_f64, write_f64 -> f64
55+
read_f64, write_f64 -> f64,
56+
read_u128, write_u128 -> u128
5657
}

src/uu/od/src/formatter_item_info.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use std::fmt;
1212
pub enum FormatWriter {
1313
IntWriter(fn(u64) -> String),
1414
FloatWriter(fn(f64) -> String),
15+
LongDoubleWriter(fn(f64) -> String), // On most platforms, long double is f64 or emulated
1516
BFloatWriter(fn(f64) -> String),
1617
MultibyteWriter(fn(&[u8]) -> String),
1718
}
@@ -27,6 +28,10 @@ impl fmt::Debug for FormatWriter {
2728
f.write_str("FloatWriter:")?;
2829
fmt::Pointer::fmt(p, f)
2930
}
31+
Self::LongDoubleWriter(ref p) => {
32+
f.write_str("LongDoubleWriter:")?;
33+
fmt::Pointer::fmt(p, f)
34+
}
3035
Self::BFloatWriter(ref p) => {
3136
f.write_str("BFloatWriter:")?;
3237
fmt::Pointer::fmt(p, f)

src/uu/od/src/input_decoder.rs

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// For the full copyright and license information, please view the LICENSE
44
// file that was distributed with this source code.
55

6-
// spell-checker:ignore bfloat multifile
6+
// spell-checker:ignore bfloat multifile mant
77

88
use half::{bf16, f16};
99
use std::io;
@@ -165,6 +165,61 @@ impl MemoryDecoder<'_> {
165165
let val = f32::from(bf16::from_bits(bits));
166166
f64::from(val)
167167
}
168+
169+
/// Returns a long double from the internal buffer at position `start`.
170+
/// We read 16 bytes as u128 (respecting endianness) and convert to f64.
171+
/// This ensures that endianness swapping works correctly even if we lose precision.
172+
pub fn read_long_double(&self, start: usize) -> f64 {
173+
let bits = self.byte_order.read_u128(&self.data[start..start + 16]);
174+
u128_to_f64(bits)
175+
}
176+
}
177+
178+
fn u128_to_f64(u: u128) -> f64 {
179+
let sign = (u >> 127) as u64;
180+
let exp = ((u >> 112) & 0x7FFF) as u64;
181+
let mant = u & ((1 << 112) - 1);
182+
183+
if exp == 0x7FFF {
184+
// Infinity or NaN
185+
if mant == 0 {
186+
if sign == 0 {
187+
f64::INFINITY
188+
} else {
189+
f64::NEG_INFINITY
190+
}
191+
} else {
192+
f64::NAN
193+
}
194+
} else if exp == 0 {
195+
// Subnormal or zero
196+
if mant == 0 {
197+
if sign == 0 { 0.0 } else { -0.0 }
198+
} else {
199+
// Subnormal f128 is too small for f64, flush to zero
200+
if sign == 0 { 0.0 } else { -0.0 }
201+
}
202+
} else {
203+
// Normal
204+
let new_exp = exp as i64 - 16383 + 1023;
205+
if new_exp >= 2047 {
206+
// Overflow to infinity
207+
if sign == 0 {
208+
f64::INFINITY
209+
} else {
210+
f64::NEG_INFINITY
211+
}
212+
} else if new_exp <= 0 {
213+
// Underflow to zero
214+
if sign == 0 { 0.0 } else { -0.0 }
215+
} else {
216+
// Normal f64
217+
// Mantissa: take top 52 bits of 112-bit mantissa
218+
let new_mant = (mant >> (112 - 52)) as u64;
219+
let bits = (sign << 63) | ((new_exp as u64) << 52) | new_mant;
220+
f64::from_bits(bits)
221+
}
222+
}
168223
}
169224

170225
#[cfg(test)]

src/uu/od/src/multifile_reader.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,13 @@ impl MultifileReader<'_> {
8787
// print an error at the time that the file is needed,
8888
// then move to the next file.
8989
// This matches the behavior of the original `od`
90-
show_error!("{}: {e}", fname.maybe_quote());
90+
// Format error without OS error code to match GNU od
91+
let error_msg = match e.kind() {
92+
io::ErrorKind::NotFound => "No such file or directory",
93+
io::ErrorKind::PermissionDenied => "Permission denied",
94+
_ => "I/O error",
95+
};
96+
show_error!("{}: {}", fname.maybe_quote().external(true), error_msg);
9197
self.any_err = true;
9298
}
9399
}

src/uu/od/src/od.rs

Lines changed: 63 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,19 @@ struct OdOptions {
8080
}
8181

8282
/// Helper function to parse bytes with error handling
83-
fn parse_bytes_option(matches: &ArgMatches, option_name: &str) -> UResult<Option<u64>> {
83+
fn parse_bytes_option(
84+
matches: &ArgMatches,
85+
args: &[String],
86+
option_name: &str,
87+
short: Option<char>,
88+
) -> UResult<Option<u64>> {
8489
match matches.get_one::<String>(option_name) {
8590
None => Ok(None),
8691
Some(s) => match parse_number_of_bytes(s) {
8792
Ok(n) => Ok(Some(n)),
8893
Err(e) => Err(USimpleError::new(
8994
1,
90-
format_error_message(&e, s, option_name),
95+
format_error_message(&e, s, &option_display_name(args, option_name, short)),
9196
)),
9297
},
9398
}
@@ -110,12 +115,12 @@ impl OdOptions {
110115
ByteOrder::Native
111116
};
112117

113-
let mut skip_bytes = parse_bytes_option(matches, options::SKIP_BYTES)?.unwrap_or(0);
118+
let mut skip_bytes =
119+
parse_bytes_option(matches, args, options::SKIP_BYTES, Some('j'))?.unwrap_or(0);
114120

115121
let mut label: Option<u64> = None;
116122

117-
let parsed_input = parse_inputs(matches)
118-
.map_err(|e| USimpleError::new(1, translate!("od-error-invalid-inputs", "msg" => e)))?;
123+
let parsed_input = parse_inputs(matches).map_err(|e| USimpleError::new(1, e))?;
119124
let input_strings = match parsed_input {
120125
CommandLineInputs::FileNames(v) => v,
121126
CommandLineInputs::FileAndOffset((f, s, l)) => {
@@ -131,16 +136,30 @@ impl OdOptions {
131136
None => 16,
132137
Some(s) => {
133138
if matches.value_source(options::WIDTH) == Some(ValueSource::CommandLine) {
134-
match parse_number_of_bytes(s) {
135-
Ok(n) => usize::try_from(n)
136-
.map_err(|_| USimpleError::new(1, format!("‘{s}‘ is too large")))?,
137-
Err(e) => {
138-
return Err(USimpleError::new(
139-
1,
140-
format_error_message(&e, s, options::WIDTH),
141-
));
142-
}
139+
let width_display = option_display_name(args, options::WIDTH, Some('w'));
140+
let parsed = parse_number_of_bytes(s).map_err(|e| {
141+
USimpleError::new(1, format_error_message(&e, s, &width_display))
142+
})?;
143+
if parsed == 0 {
144+
return Err(USimpleError::new(
145+
1,
146+
translate!(
147+
"od-error-invalid-argument",
148+
"option" => width_display.clone(),
149+
"value" => s.quote()
150+
),
151+
));
143152
}
153+
usize::try_from(parsed).map_err(|_| {
154+
USimpleError::new(
155+
1,
156+
translate!(
157+
"od-error-argument-too-large",
158+
"option" => width_display.clone(),
159+
"value" => s.quote()
160+
),
161+
)
162+
})?
144163
} else {
145164
16
146165
}
@@ -160,9 +179,9 @@ impl OdOptions {
160179

161180
let output_duplicates = matches.get_flag(options::OUTPUT_DUPLICATES);
162181

163-
let read_bytes = parse_bytes_option(matches, options::READ_BYTES)?;
182+
let read_bytes = parse_bytes_option(matches, args, options::READ_BYTES, Some('N'))?;
164183

165-
let string_min_length = match parse_bytes_option(matches, options::STRINGS)? {
184+
let string_min_length = match parse_bytes_option(matches, args, options::STRINGS, Some('S'))? {
166185
None => None,
167186
Some(n) => Some(usize::try_from(n).map_err(|_| {
168187
USimpleError::new(
@@ -491,7 +510,9 @@ where
491510
let length = memory_decoder.length();
492511

493512
if length == 0 {
494-
input_offset.print_final_offset();
513+
if !input_decoder.has_error() {
514+
input_offset.print_final_offset();
515+
}
495516
break;
496517
}
497518

@@ -669,6 +690,10 @@ fn print_bytes(prefix: &str, input_decoder: &MemoryDecoder, output_info: &Output
669690
let p = input_decoder.read_float(b, f.formatter_item_info.byte_size);
670691
output_text.push_str(&func(p));
671692
}
693+
FormatWriter::LongDoubleWriter(func) => {
694+
let p = input_decoder.read_long_double(b);
695+
output_text.push_str(&func(p));
696+
}
672697
FormatWriter::BFloatWriter(func) => {
673698
let p = input_decoder.read_bfloat(b);
674699
output_text.push_str(&func(p));
@@ -745,6 +770,27 @@ impl<R: HasError> HasError for BufReader<R> {
745770
}
746771
}
747772

773+
fn option_display_name(args: &[String], option_name: &str, short: Option<char>) -> String {
774+
let long_form = format!("--{option_name}");
775+
let long_form_with_eq = format!("{long_form}=");
776+
if let Some(short_char) = short {
777+
let short_form = format!("-{short_char}");
778+
for arg in args.iter().skip(1) {
779+
if !arg.starts_with("--") && arg.starts_with(&short_form) {
780+
return short_form;
781+
}
782+
}
783+
for arg in args.iter().skip(1) {
784+
if arg == &long_form || arg.starts_with(&long_form_with_eq) {
785+
return long_form;
786+
}
787+
}
788+
short_form
789+
} else {
790+
long_form
791+
}
792+
}
793+
748794
fn format_error_message(error: &ParseSizeError, s: &str, option: &str) -> String {
749795
// NOTE:
750796
// GNU's od echos affected flag, -N or --read-bytes (-j or --skip-bytes, etc.), depending user's selection

0 commit comments

Comments
 (0)