Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 34 additions & 14 deletions src/uu/numfmt/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -787,20 +787,40 @@ fn split_bytes<'a>(input: &'a [u8], delim: &'a [u8]) -> impl Iterator<Item = &'a
})
}

pub fn write_formatted_with_delimiter<W: std::io::Write + ?Sized>(
#[derive(Debug)]
pub(crate) enum WriteFormattedError {
Format(String),
Io(std::io::Error),
}

impl From<String> for WriteFormattedError {
fn from(error: String) -> Self {
Self::Format(error)
}
}

impl From<std::io::Error> for WriteFormattedError {
fn from(error: std::io::Error) -> Self {
Self::Io(error)
}
}

type WriteFormattedResult<T> = std::result::Result<T, WriteFormattedError>;

pub(crate) fn write_formatted_with_delimiter<W: std::io::Write + ?Sized>(
writer: &mut W,
input: &[u8],
options: &NumfmtOptions,
eol: Option<u8>,
) -> Result<()> {
) -> WriteFormattedResult<()> {
let delimiter = options.delimiter.as_deref().unwrap();

for (n, field) in (1..).zip(split_bytes(input, delimiter)) {
let field_selected = uucore::ranges::contain(&options.fields, n);

// add delimiter before second and subsequent fields
if n > 1 {
writer.write_all(delimiter).unwrap();
writer.write_all(delimiter)?;
}

if field_selected {
Expand All @@ -809,26 +829,26 @@ pub fn write_formatted_with_delimiter<W: std::io::Write + ?Sized>(
.map_err(|_| translate!("numfmt-error-invalid-number", "input" => escape_line(field).quote()))?
.trim_start();
let formatted = format_string(field_str, options, None)?;
writer.write_all(formatted.as_bytes()).unwrap();
writer.write_all(formatted.as_bytes())?;
} else {
// add unselected field without conversion
writer.write_all(field).unwrap();
writer.write_all(field)?;
}
}

if let Some(eol) = eol {
writer.write_all(&[eol]).unwrap();
writer.write_all(&[eol])?;
}

Ok(())
}

pub fn write_formatted_with_whitespace<W: std::io::Write + ?Sized>(
pub(crate) fn write_formatted_with_whitespace<W: std::io::Write + ?Sized>(
writer: &mut W,
s: &str,
options: &NumfmtOptions,
eol: Option<u8>,
) -> Result<()> {
) -> WriteFormattedResult<()> {
for (n, (prefix, field)) in (1..).zip(WhitespaceSplitter {
s: Some(s),
options,
Expand All @@ -840,7 +860,7 @@ pub fn write_formatted_with_whitespace<W: std::io::Write + ?Sized>(

// add delimiter before second and subsequent fields
let prefix = if n > 1 {
writer.write_all(b" ").unwrap();
writer.write_all(b" ")?;
&prefix[1..]
} else {
prefix
Expand All @@ -853,23 +873,23 @@ pub fn write_formatted_with_whitespace<W: std::io::Write + ?Sized>(
};

let formatted = format_string(field, options, implicit_padding)?;
writer.write_all(formatted.as_bytes()).unwrap();
writer.write_all(formatted.as_bytes())?;
} else {
// the -z option converts an initial \n into a space
let prefix = if options.zero_terminated && prefix.starts_with('\n') {
writer.write_all(b" ").unwrap();
writer.write_all(b" ")?;
&prefix[1..]
} else {
prefix
};
// add unselected field without conversion
writer.write_all(prefix.as_bytes()).unwrap();
writer.write_all(field.as_bytes()).unwrap();
writer.write_all(prefix.as_bytes())?;
writer.write_all(field.as_bytes())?;
}
}

if let Some(eol) = eol {
writer.write_all(&[eol]).unwrap();
writer.write_all(&[eol])?;
}

Ok(())
Expand Down
39 changes: 27 additions & 12 deletions src/uu/numfmt/src/numfmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@
// spell-checker:ignore behavior

use crate::errors::NumfmtError;
use crate::format::{escape_line, write_formatted_with_delimiter, write_formatted_with_whitespace};
use crate::format::{
WriteFormattedError, escape_line, write_formatted_with_delimiter,
write_formatted_with_whitespace,
};
use crate::options::{
DEBUG, DELIMITER, FIELD, FIELD_DEFAULT, FORMAT, FROM, FROM_DEFAULT, FROM_UNIT,
FROM_UNIT_DEFAULT, FormatOptions, GROUPING, HEADER, HEADER_DEFAULT, INVALID, InvalidModes,
Expand All @@ -19,7 +22,7 @@ use std::io::{BufRead, Write as _, stderr};
use std::str::FromStr;

use uucore::display::Quotable;
use uucore::error::UResult;
use uucore::error::{FromIo, UResult};
use uucore::i18n::decimal::locale_grouping_separator;
use uucore::parser::parse_size::{IEC_BASES, SI_BASES};
use uucore::parser::shortcut_value_parser::ShortcutValueParser;
Expand All @@ -46,6 +49,12 @@ fn is_scientific(input: &[u8]) -> bool {
false
}

fn write_output<W: std::io::Write + ?Sized>(writer: &mut W, buf: &[u8]) -> UResult<()> {
writer
.write_all(buf)
.map_err_context(|| "write error".into())
}

/// Format a single line and write it, handling `--invalid` error modes.
///
/// Returns `true` if the line contained invalid input (only possible in
Expand Down Expand Up @@ -74,22 +83,28 @@ fn format_and_write<W: std::io::Write>(
match std::str::from_utf8(line) {
Ok(s) => {
if is_scientific(s.as_bytes()) {
Err(format!(
Err(WriteFormattedError::Format(format!(
"invalid suffix in input: '{}'",
String::from_utf8_lossy(line)
))
)))
} else {
write_formatted_with_whitespace(dest, s, options, eol)
}
}
Err(_) => Err(translate!(
Err(_) => Err(WriteFormattedError::Format(translate!(
"numfmt-error-invalid-number",
"input" => escape_line(line).quote()
)),
))),
}
};

if let Err(msg) = result {
if let Err(error) = result {
let msg = match error {
WriteFormattedError::Format(msg) => msg,
WriteFormattedError::Io(error) => {
return Err(error.map_err_context(|| "write error".into()));
}
};
match options.invalid {
InvalidModes::Abort => {
return Err(Box::new(NumfmtError::FormattingError(msg)));
Expand All @@ -103,15 +118,15 @@ fn format_and_write<W: std::io::Write>(
InvalidModes::Ignore => {}
}
// On error, echo the original line unchanged.
writer.write_all(input_line)?;
write_output(writer, input_line)?;
if let Some(eol) = eol {
writer.write_all(&[eol])?;
write_output(writer, &[eol])?;
}
return Ok(true);
}

if buffer_output {
writer.write_all(&buf)?;
write_output(writer, &buf)?;
}
Ok(false)
}
Expand Down Expand Up @@ -160,9 +175,9 @@ fn handle_buffer<R: BufRead>(mut input: R, options: &NumfmtOptions) -> UResult<b

if line_idx < options.header {
// Pass header lines through unchanged.
stdout.write_all(line)?;
write_output(&mut stdout, line)?;
if let Some(t) = eol {
stdout.write_all(&[t])?;
write_output(&mut stdout, &[t])?;
}
} else {
saw_invalid |= format_and_write(&mut stdout, line, options, eol)?;
Expand Down
16 changes: 16 additions & 0 deletions tests/by-util/test_numfmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,22 @@ fn test_invalid_arg() {
new_ucmd!().arg("--definitely-invalid").fails_with_code(1);
}

#[cfg(target_os = "linux")]
#[test]
fn test_output_write_error_reports_without_panic() {
let dev_full = std::fs::OpenOptions::new()
.write(true)
.open("/dev/full")
.expect("/dev/full should be available on Linux");

new_ucmd!()
.arg("1024")
.set_stdout(std::process::Stdio::from(dev_full))
.fails_with_code(1)
.stderr_contains("numfmt: write error: No space left on device")
.stderr_does_not_contain("panicked");
Comment on lines +27 to +28

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A detail: I would check for the exact error output:

Suggested change
.stderr_contains("numfmt: write error: No space left on device")
.stderr_does_not_contain("panicked");
.stderr_is("numfmt: write error: No space left on device\n");

}

// This test failed when fixing #11653.
// Add a `--` separator to ensure floats are not rounded(it match the gnu pattern).
#[test]
Expand Down
Loading