refactor(od): extract output writer parameter for better testability

- Added generic `W: Write` parameter to `print_final_offset()` method
- Updated all `odfunc()` calls to pass output writer explicitly
- Replaced direct `println!()` calls with `writeln!(out, ...)` throughout
- Enables dependency injection for output streams in tests
- Maintains existing functionality while improving testability
This commit is contained in:
mattsu
2026-01-30 19:47:23 +09:00
parent 12193c52c6
commit 3cd4ff1dd0
3 changed files with 52 additions and 20 deletions
+5 -2
View File
@@ -2,6 +2,8 @@
//
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
use std::io::{self, Write};
#[derive(Copy, Clone, Debug, Eq, PartialEq)]
pub enum Radix {
Decimal,
@@ -60,10 +62,11 @@ impl InputOffset {
/// Prints the byte offset followed by a newline, or nothing at all if
/// both `Radix::NoPrefix` was set and no label (--traditional) is used.
pub fn print_final_offset(&self) {
pub fn print_final_offset<W: Write>(&self, out: &mut W) -> io::Result<()> {
if self.radix != Radix::NoPrefix || self.label.is_some() {
println!("{}", self.format_byte_offset());
writeln!(out, "{}", self.format_byte_offset())?;
}
Ok(())
}
}
+30 -18
View File
@@ -26,7 +26,7 @@ mod prn_int;
use std::cmp;
use std::fmt::Write;
use std::io::{BufReader, Read};
use std::io::{BufReader, Read, Write as IoWrite};
use crate::byteorder_io::ByteOrder;
use crate::formatter_item_info::FormatWriter;
@@ -246,6 +246,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
let clap_matches = uucore::clap_localization::handle_clap_result(clap_opts, &args)?;
let od_options = OdOptions::new(&clap_matches, &args)?;
let mut out = std::io::stdout().lock();
// Check if we're in strings mode
if let Some(min_length) = od_options.string_min_length {
@@ -255,6 +256,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
od_options.read_bytes,
min_length,
od_options.radix,
&mut out,
)
} else {
let mut input_offset =
@@ -278,7 +280,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
od_options.output_duplicates,
);
odfunc(&mut input_offset, &mut input_decoder, &output_info)
odfunc(&mut input_offset, &mut input_decoder, &output_info, &mut out)
}
}
@@ -490,13 +492,15 @@ pub fn uu_app() -> Command {
}
/// Loops through the input line by line, calling `print_bytes` to take care of the output.
fn odfunc<I>(
fn odfunc<I, W>(
input_offset: &mut InputOffset,
input_decoder: &mut InputDecoder<I>,
output_info: &OutputInfo,
out: &mut W,
) -> UResult<()>
where
I: PeekRead + HasError,
W: IoWrite,
{
let mut duplicate_line = false;
let mut previous_bytes: Vec<u8> = Vec::new();
@@ -511,7 +515,7 @@ where
if length == 0 {
if !input_decoder.has_error() {
input_offset.print_final_offset();
input_offset.print_final_offset(out)?;
}
break;
}
@@ -533,7 +537,7 @@ where
{
if !duplicate_line {
duplicate_line = true;
println!("*");
writeln!(out, "*")?;
}
} else {
duplicate_line = false;
@@ -546,14 +550,15 @@ where
&input_offset.format_byte_offset(),
&memory_decoder,
output_info,
);
out,
)?;
}
input_offset.increase_position(length as u64);
}
Err(e) => {
show_error!("{e}");
input_offset.print_final_offset();
input_offset.print_final_offset(out)?;
return Err(1.into());
}
}
@@ -573,6 +578,7 @@ fn extract_strings_from_input(
read_bytes: Option<u64>,
min_length: usize,
radix: Radix,
out: &mut impl IoWrite,
) -> UResult<()> {
let inputs = map_input_strings(input_strings);
let mut mf = MultifileReader::new(inputs);
@@ -590,13 +596,13 @@ fn extract_strings_from_input(
}
// Helper function to format and print a string
let print_string = |offset: u64, string: &[u8]| {
let mut print_string = |offset: u64, string: &[u8]| -> std::io::Result<()> {
let string_content = String::from_utf8_lossy(string);
match radix {
Radix::NoPrefix => println!("{string_content}"),
Radix::Decimal => println!("{offset:07} {string_content}"),
Radix::Hexadecimal => println!("{offset:07x} {string_content}"),
Radix::Octal => println!("{offset:07o} {string_content}"),
Radix::NoPrefix => writeln!(out, "{string_content}"),
Radix::Decimal => writeln!(out, "{offset:07} {string_content}"),
Radix::Hexadecimal => writeln!(out, "{offset:07x} {string_content}"),
Radix::Octal => writeln!(out, "{offset:07o} {string_content}"),
}
};
@@ -613,7 +619,7 @@ fn extract_strings_from_input(
// Special case: when -N limit is reached with a pending string
// that meets min_length, output it even without null terminator
if current_string.len() >= min_length {
print_string(string_start_offset, &current_string);
print_string(string_start_offset, &current_string)?;
}
break;
}
@@ -636,7 +642,7 @@ fn extract_strings_from_input(
// Either null terminator or non-printable character
if byte == 0 && current_string.len() >= min_length {
// Null terminator found with valid string
print_string(string_start_offset, &current_string);
print_string(string_start_offset, &current_string)?;
}
current_string.clear();
}
@@ -666,7 +672,12 @@ fn extract_strings_from_input(
}
/// Outputs a single line of input, into one or more lines human readable output.
fn print_bytes(prefix: &str, input_decoder: &MemoryDecoder, output_info: &OutputInfo) {
fn print_bytes<W: IoWrite>(
prefix: &str,
input_decoder: &MemoryDecoder,
output_info: &OutputInfo,
out: &mut W,
) -> std::io::Result<()> {
let mut first = true; // First line of a multi-format raster.
for f in output_info.spaced_formatters_iter() {
let mut output_text = String::new();
@@ -720,16 +731,17 @@ fn print_bytes(prefix: &str, input_decoder: &MemoryDecoder, output_info: &Output
}
if first {
print!("{prefix}"); // print offset
write!(out, "{prefix}")?; // print offset
// if printing in multiple formats offset is printed only once
first = false;
} else {
// this takes the space of the file offset on subsequent
// lines of multi-format rasters.
print!("{:>width$}", "", width = prefix.chars().count());
write!(out, "{:>width$}", "", width = prefix.chars().count())?;
}
println!("{output_text}");
writeln!(out, "{output_text}")?;
}
Ok(())
}
/// Helper function to convert input strings to InputSource
+17
View File
@@ -1259,3 +1259,20 @@ fn test_od_eintr_handling() {
.no_stderr()
.stdout_contains("e"); // Should contain 'e' from "ello"
}
// Regression test: od should handle write errors to /dev/full without aborting.
#[test]
#[cfg(target_os = "linux")]
fn test_write_error_dev_full() {
use std::fs::File;
let dev_full = File::create("/dev/full").expect("Failed to open /dev/full");
new_ucmd!()
.arg("-An")
.pipe_in("abcd")
.set_stdout(dev_full)
.fails()
.code_is(1)
.stderr_contains("No space left on device");
}