From 9972cd1327f1af41c7f17f4211a49f9f1e7cc463 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Wed, 25 Aug 2021 15:57:33 +0200 Subject: [PATCH] wc: Report counts and failures correctly If reading fails midway through then a count should be reported for what could be read. If opening a file fails then no count should be reported. The exit code shouldn't report the number of failures, that's fragile in case of many failures. --- src/uu/wc/src/count_fast.rs | 18 +++++---- src/uu/wc/src/wc.rs | 80 ++++++++++++++++++++++++++----------- 2 files changed, 66 insertions(+), 32 deletions(-) diff --git a/src/uu/wc/src/count_fast.rs b/src/uu/wc/src/count_fast.rs index f2081be8c..deaa890fd 100644 --- a/src/uu/wc/src/count_fast.rs +++ b/src/uu/wc/src/count_fast.rs @@ -88,7 +88,7 @@ fn count_bytes_using_splice(fd: RawFd) -> Result { /// 3. Otherwise, we just read normally, but without the overhead of counting /// other things such as lines and words. #[inline] -pub(crate) fn count_bytes_fast(handle: &mut T) -> io::Result { +pub(crate) fn count_bytes_fast(handle: &mut T) -> (usize, Option) { let mut byte_count = 0; #[cfg(unix)] @@ -98,7 +98,7 @@ pub(crate) fn count_bytes_fast(handle: &mut T) -> io::Result(handle: &mut T) -> io::Result return Ok(n), + Ok(n) => return (n, None), Err(n) => byte_count = n, } } @@ -118,28 +118,30 @@ pub(crate) fn count_bytes_fast(handle: &mut T) -> io::Result return Ok(byte_count), + Ok(0) => return (byte_count, None), Ok(n) => { byte_count += n; } Err(ref e) if e.kind() == ErrorKind::Interrupted => continue, - Err(e) => return Err(e), + Err(e) => return (byte_count, Some(e)), } } } -pub(crate) fn count_bytes_and_lines_fast(handle: &mut R) -> io::Result { +pub(crate) fn count_bytes_and_lines_fast( + handle: &mut R, +) -> (WordCount, Option) { let mut total = WordCount::default(); let mut buf = [0; BUF_SIZE]; loop { match handle.read(&mut buf) { - Ok(0) => return Ok(total), + Ok(0) => return (total, None), Ok(n) => { total.bytes += n; total.lines += bytecount::count(&buf[..n], b'\n'); } Err(ref e) if e.kind() == ErrorKind::Interrupted => continue, - Err(e) => return Err(e), + Err(e) => return (total, Some(e)), } } } diff --git a/src/uu/wc/src/wc.rs b/src/uu/wc/src/wc.rs index b4649ab91..8f219a1e9 100644 --- a/src/uu/wc/src/wc.rs +++ b/src/uu/wc/src/wc.rs @@ -202,17 +202,21 @@ pub fn uu_app() -> App<'static, 'static> { fn word_count_from_reader( mut reader: T, settings: &Settings, -) -> io::Result { +) -> (WordCount, Option) { let only_count_bytes = settings.show_bytes && (!(settings.show_chars || settings.show_lines || settings.show_max_line_length || settings.show_words)); if only_count_bytes { - return Ok(WordCount { - bytes: count_bytes_fast(&mut reader)?, - ..WordCount::default() - }); + let (bytes, error) = count_bytes_fast(&mut reader); + return ( + WordCount { + bytes, + ..WordCount::default() + }, + error, + ); } // we do not need to decode the byte stream if we're only counting bytes/newlines @@ -268,27 +272,47 @@ fn word_count_from_reader( total.bytes += bytes.len(); } Err(BufReadDecoderError::Io(e)) => { - return Err(e); + return (total, Some(e)); } } } total.max_line_length = max(current_len, total.max_line_length); - Ok(total) + (total, None) } -fn word_count_from_input(input: &Input, settings: &Settings) -> io::Result { +enum CountResult { + /// Nothing went wrong. + Success(WordCount), + /// Managed to open but failed to read. + Interrupted(WordCount, io::Error), + /// Didn't even manage to open. + Failure(io::Error), +} + +/// If we fail opening a file we only show the error. If we fail reading it +/// we show a count for what we managed to read. +/// +/// Therefore the reading implementations always return a total and sometimes +/// return an error: (WordCount, Option). +fn word_count_from_input(input: &Input, settings: &Settings) -> CountResult { match input { Input::Stdin(_) => { let stdin = io::stdin(); let stdin_lock = stdin.lock(); - word_count_from_reader(stdin_lock, settings) - } - Input::Path(path) => { - let file = File::open(path)?; - word_count_from_reader(file, settings) + match word_count_from_reader(stdin_lock, settings) { + (total, Some(error)) => CountResult::Interrupted(total, error), + (total, None) => CountResult::Success(total), + } } + Input::Path(path) => match File::open(path) { + Err(error) => CountResult::Failure(error), + Ok(file) => match word_count_from_reader(file, settings) { + (total, Some(error)) => CountResult::Interrupted(total, error), + (total, None) => CountResult::Success(total), + }, + }, } } @@ -390,7 +414,7 @@ fn wc(inputs: Vec, settings: &Settings) -> Result<(), u32> { // The width is the number of digits needed to print the number of // bytes in the largest file. This is true regardless of whether // the `settings` indicate that the bytes will be displayed. - let mut error_count = 0; + let mut failure = false; let max_width = max_width(&inputs); let mut total_word_count = WordCount::default(); @@ -398,11 +422,19 @@ fn wc(inputs: Vec, settings: &Settings) -> Result<(), u32> { let num_inputs = inputs.len(); for input in &inputs { - let word_count = word_count_from_input(input, settings).unwrap_or_else(|err| { - show_error(input, err); - error_count += 1; - WordCount::default() - }); + let word_count = match word_count_from_input(input, settings) { + CountResult::Success(word_count) => word_count, + CountResult::Interrupted(word_count, error) => { + show_error(input, error); + failure = true; + word_count + } + CountResult::Failure(error) => { + show_error(input, error); + failure = true; + continue; + } + }; total_word_count += word_count; let result = word_count.with_title(input.to_title()); if let Err(err) = print_stats(settings, &result, max_width) { @@ -411,7 +443,7 @@ fn wc(inputs: Vec, settings: &Settings) -> Result<(), u32> { result.title.unwrap_or_else(|| "".as_ref()).display(), err ); - error_count += 1; + failure = true; } } @@ -419,14 +451,14 @@ fn wc(inputs: Vec, settings: &Settings) -> Result<(), u32> { let total_result = total_word_count.with_title(Some("total".as_ref())); if let Err(err) = print_stats(settings, &total_result, max_width) { show_warning!("failed to print total: {}", err); - error_count += 1; + failure = true; } } - if error_count == 0 { - Ok(()) + if failure { + Err(1) } else { - Err(error_count) + Ok(()) } }