Skip to content

Commit 18d2ee7

Browse files
committed
feat: add write error handling for cat output
- Introduce WriteError variant in CatError enum to specifically handle stdout write failures - Add map_write_err helper to wrap io::Result into CatResult with WriteError - Update cat_files to catch WriteError early and report using localized message - Wrap write_all and flush calls in write_fast and write_lines with map_write_err - Add localized error messages for "write error" in en-US and fr-FR locales This improves error reporting for I/O issues like broken pipes during output.
1 parent 44e0bba commit 18d2ee7

File tree

5 files changed

+67
-39
lines changed

5 files changed

+67
-39
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,4 @@ cat-error-is-directory = Is a directory
2020
cat-error-input-file-is-output-file = input file is output file
2121
cat-error-too-many-symbolic-links = Too many levels of symbolic links
2222
cat-error-no-such-device-or-address = No such device or address
23+
cat-error-write-error = write error

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,4 @@ cat-error-is-directory = Est un répertoire
2020
cat-error-input-file-is-output-file = le fichier d'entrée est le fichier de sortie
2121
cat-error-too-many-symbolic-links = Trop de niveaux de liens symboliques
2222
cat-error-no-such-device-or-address = Aucun appareil ou adresse de ce type
23+
cat-error-write-error = erreur d'écriture

src/uu/cat/src/cat.rs

Lines changed: 57 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use std::os::fd::AsFd;
1919
use std::os::unix::fs::FileTypeExt;
2020
use thiserror::Error;
2121
use uucore::display::Quotable;
22-
use uucore::error::UResult;
22+
use uucore::error::{UIoError, UResult};
2323
#[cfg(not(target_os = "windows"))]
2424
use uucore::libc;
2525
use uucore::translate;
@@ -86,6 +86,9 @@ enum CatError {
8686
/// Wrapper around `io::Error`
8787
#[error("{0}")]
8888
Io(#[from] io::Error),
89+
/// Wrapper around `io::Error` for stdout writes
90+
#[error("{0}")]
91+
WriteError(io::Error),
8992
/// Wrapper around `nix::Error`
9093
#[cfg(any(target_os = "linux", target_os = "android"))]
9194
#[error("{0}")]
@@ -109,6 +112,11 @@ enum CatError {
109112

110113
type CatResult<T> = Result<T, CatError>;
111114

115+
#[inline]
116+
fn map_write_err<T>(res: io::Result<T>) -> CatResult<T> {
117+
res.map_err(CatError::WriteError)
118+
}
119+
112120
#[derive(PartialEq)]
113121
enum NumberingMode {
114122
None,
@@ -416,15 +424,30 @@ fn cat_files(files: &[OsString], options: &OutputOptions) -> UResult<()> {
416424
one_blank_kept: false,
417425
};
418426
let mut error_messages: Vec<String> = Vec::new();
427+
let mut write_error: Option<io::Error> = None;
419428

420429
for path in files {
421-
if let Err(err) = cat_path(path, options, &mut state) {
422-
error_messages.push(format!("{}: {err}", path.maybe_quote()));
430+
match cat_path(path, options, &mut state) {
431+
Ok(()) => {}
432+
Err(CatError::WriteError(err)) => {
433+
write_error = Some(err);
434+
break;
435+
}
436+
Err(err) => {
437+
error_messages.push(format!("{}: {err}", path.maybe_quote()));
438+
}
423439
}
424440
}
425441
if state.skipped_carriage_return {
426442
print!("\r");
427443
}
444+
if let Some(err) = write_error {
445+
let message = UIoError::from(err);
446+
error_messages.push(format!(
447+
"{}: {message}",
448+
translate!("cat-error-write-error")
449+
));
450+
}
428451
if error_messages.is_empty() {
429452
Ok(())
430453
} else {
@@ -505,9 +528,11 @@ fn write_fast<R: FdReadable>(handle: &mut InputHandle<R>) -> CatResult<()> {
505528
if n == 0 {
506529
break;
507530
}
508-
stdout_lock
509-
.write_all(&buf[..n])
510-
.inspect_err(handle_broken_pipe)?;
531+
map_write_err(
532+
stdout_lock
533+
.write_all(&buf[..n])
534+
.inspect_err(handle_broken_pipe),
535+
)?;
511536
}
512537
Err(e) if e.kind() == ErrorKind::Interrupted => continue,
513538
Err(e) => return Err(e.into()),
@@ -519,7 +544,7 @@ fn write_fast<R: FdReadable>(handle: &mut InputHandle<R>) -> CatResult<()> {
519544
// that will succeed, data pushed through splice will be output before
520545
// the data buffered in stdout.lock. Therefore additional explicit flush
521546
// is required here.
522-
stdout_lock.flush().inspect_err(handle_broken_pipe)?;
547+
map_write_err(stdout_lock.flush().inspect_err(handle_broken_pipe))?;
523548
Ok(())
524549
}
525550

@@ -554,13 +579,13 @@ fn write_lines<R: FdReadable>(
554579
continue;
555580
}
556581
if state.skipped_carriage_return {
557-
writer.write_all(b"\r")?;
582+
map_write_err(writer.write_all(b"\r"))?;
558583
state.skipped_carriage_return = false;
559584
state.at_line_start = false;
560585
}
561586
state.one_blank_kept = false;
562587
if state.at_line_start && options.number != NumberingMode::None {
563-
state.line_number.write(&mut writer)?;
588+
map_write_err(state.line_number.write(&mut writer))?;
564589
state.line_number.increment();
565590
}
566591

@@ -593,7 +618,7 @@ fn write_lines<R: FdReadable>(
593618
// and not be buffered internally to the `cat` process.
594619
// Hence it's necessary to flush our buffer before every time we could potentially block
595620
// on a `std::io::Read::read` call.
596-
writer.flush().inspect_err(handle_broken_pipe)?;
621+
map_write_err(writer.flush().inspect_err(handle_broken_pipe))?;
597622
}
598623

599624
Ok(())
@@ -608,9 +633,9 @@ fn write_new_line<W: Write>(
608633
) -> CatResult<()> {
609634
if state.skipped_carriage_return {
610635
if options.show_ends {
611-
writer.write_all(b"^M")?;
636+
map_write_err(writer.write_all(b"^M"))?;
612637
} else {
613-
writer.write_all(b"\r")?;
638+
map_write_err(writer.write_all(b"\r"))?;
614639
}
615640
state.skipped_carriage_return = false;
616641

@@ -620,19 +645,15 @@ fn write_new_line<W: Write>(
620645
if !state.at_line_start || !options.squeeze_blank || !state.one_blank_kept {
621646
state.one_blank_kept = true;
622647
if state.at_line_start && options.number == NumberingMode::All {
623-
state.line_number.write(writer)?;
648+
map_write_err(state.line_number.write(writer))?;
624649
state.line_number.increment();
625650
}
626651
write_end_of_line(writer, options.end_of_line().as_bytes(), is_interactive)?;
627652
}
628653
Ok(())
629654
}
630655

631-
fn write_end<W: Write>(
632-
writer: &mut W,
633-
in_buf: &[u8],
634-
options: &OutputOptions,
635-
) -> io::Result<usize> {
656+
fn write_end<W: Write>(writer: &mut W, in_buf: &[u8], options: &OutputOptions) -> CatResult<usize> {
636657
if options.show_nonprint {
637658
write_nonprint_to_end(in_buf, writer, options.tab().as_bytes())
638659
} else if options.show_tabs {
@@ -648,31 +669,31 @@ fn write_end<W: Write>(
648669
// however, write_nonprint_to_end doesn't need to stop at \r because it will always write \r as ^M.
649670
// Return the number of written symbols
650671

651-
fn write_to_end<W: Write>(in_buf: &[u8], writer: &mut W) -> io::Result<usize> {
672+
fn write_to_end<W: Write>(in_buf: &[u8], writer: &mut W) -> CatResult<usize> {
652673
// using memchr2 significantly improves performances
653674
match memchr2(b'\n', b'\r', in_buf) {
654675
Some(p) => {
655-
writer.write_all(&in_buf[..p])?;
676+
map_write_err(writer.write_all(&in_buf[..p]))?;
656677
Ok(p)
657678
}
658679
None => {
659-
writer.write_all(in_buf)?;
680+
map_write_err(writer.write_all(in_buf))?;
660681
Ok(in_buf.len())
661682
}
662683
}
663684
}
664685

665-
fn write_tab_to_end<W: Write>(mut in_buf: &[u8], writer: &mut W) -> io::Result<usize> {
686+
fn write_tab_to_end<W: Write>(mut in_buf: &[u8], writer: &mut W) -> CatResult<usize> {
666687
let mut count = 0;
667688
loop {
668689
match in_buf
669690
.iter()
670691
.position(|c| *c == b'\n' || *c == b'\t' || *c == b'\r')
671692
{
672693
Some(p) => {
673-
writer.write_all(&in_buf[..p])?;
694+
map_write_err(writer.write_all(&in_buf[..p]))?;
674695
if in_buf[p] == b'\t' {
675-
writer.write_all(b"^I")?;
696+
map_write_err(writer.write_all(b"^I"))?;
676697
in_buf = &in_buf[p + 1..];
677698
count += p + 1;
678699
} else {
@@ -681,29 +702,29 @@ fn write_tab_to_end<W: Write>(mut in_buf: &[u8], writer: &mut W) -> io::Result<u
681702
}
682703
}
683704
None => {
684-
writer.write_all(in_buf)?;
705+
map_write_err(writer.write_all(in_buf))?;
685706
return Ok(in_buf.len() + count);
686707
}
687708
}
688709
}
689710
}
690711

691-
fn write_nonprint_to_end<W: Write>(in_buf: &[u8], writer: &mut W, tab: &[u8]) -> io::Result<usize> {
712+
fn write_nonprint_to_end<W: Write>(in_buf: &[u8], writer: &mut W, tab: &[u8]) -> CatResult<usize> {
692713
let mut count = 0;
693714

694715
for byte in in_buf.iter().copied() {
695716
if byte == b'\n' {
696717
break;
697718
}
698719
match byte {
699-
9 => writer.write_all(tab),
700-
0..=8 | 10..=31 => writer.write_all(&[b'^', byte + 64]),
701-
32..=126 => writer.write_all(&[byte]),
702-
127 => writer.write_all(b"^?"),
703-
128..=159 => writer.write_all(&[b'M', b'-', b'^', byte - 64]),
704-
160..=254 => writer.write_all(&[b'M', b'-', byte - 128]),
705-
_ => writer.write_all(b"M-^?"),
706-
}?;
720+
9 => map_write_err(writer.write_all(tab))?,
721+
0..=8 | 10..=31 => map_write_err(writer.write_all(&[b'^', byte + 64]))?,
722+
32..=126 => map_write_err(writer.write_all(&[byte]))?,
723+
127 => map_write_err(writer.write_all(b"^?"))?,
724+
128..=159 => map_write_err(writer.write_all(&[b'M', b'-', b'^', byte - 64]))?,
725+
160..=254 => map_write_err(writer.write_all(&[b'M', b'-', byte - 128]))?,
726+
_ => map_write_err(writer.write_all(b"M-^?"))?,
727+
};
707728
count += 1;
708729
}
709730
Ok(count)
@@ -714,9 +735,9 @@ fn write_end_of_line<W: Write>(
714735
end_of_line: &[u8],
715736
is_interactive: bool,
716737
) -> CatResult<()> {
717-
writer.write_all(end_of_line)?;
738+
map_write_err(writer.write_all(end_of_line))?;
718739
if is_interactive {
719-
writer.flush().inspect_err(handle_broken_pipe)?;
740+
map_write_err(writer.flush().inspect_err(handle_broken_pipe))?;
720741
}
721742
Ok(())
722743
}

src/uu/cat/src/splice.rs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
//
33
// For the full copyright and license information, please view the LICENSE
44
// file that was distributed with this source code.
5-
use super::{CatResult, FdReadable, InputHandle};
5+
use super::{CatError, CatResult, FdReadable, InputHandle};
66

77
use nix::unistd;
8+
use std::io;
89
use std::os::{fd::AsFd, unix::io::AsRawFd};
910

1011
use uucore::pipes::{pipe, splice, splice_exact};
@@ -38,7 +39,11 @@ pub(super) fn write_fast_using_splice<R: FdReadable, S: AsRawFd + AsFd>(
3839
// we can recover by copying the data that we have from the
3940
// intermediate pipe to stdout using normal read/write. Then
4041
// we tell the caller to fall back.
41-
copy_exact(&pipe_rd, write_fd, n)?;
42+
if let Err(err) = copy_exact(&pipe_rd, write_fd, n) {
43+
return Err(CatError::WriteError(io::Error::from_raw_os_error(
44+
err as i32,
45+
)));
46+
}
4247
return Ok(true);
4348
}
4449
}

tests/by-util/test_cat.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ fn test_piped_to_dev_full() {
223223
.pipe_in_fixture("alpha.txt")
224224
.ignore_stdin_write_error()
225225
.fails()
226-
.stderr_contains("No space left on device");
226+
.stderr_contains("write error: No space left on device");
227227
}
228228
}
229229
}

0 commit comments

Comments
 (0)