From 657a04f706e7d58beec9af1dda8b0f859c241bdc Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Wed, 25 Aug 2021 14:26:03 +0200 Subject: [PATCH] wc: Stricter simpler error handling Errors are now always shown with the corresponding filename. Errors are no longer converted into warnings. Previously `wc < .` would cause a loop. Checking whether something is a directory is no longer done in advance. This removes race conditions and the edge case where stdin is a directory. The custom error type is removed because io::Error is now enough. --- Cargo.lock | 1 - src/uu/wc/Cargo.toml | 1 - src/uu/wc/src/count_fast.rs | 12 ++++---- src/uu/wc/src/wc.rs | 55 ++++++++++++------------------------- tests/by-util/test_wc.rs | 4 +-- 5 files changed, 26 insertions(+), 47 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4d51b124d..a9c095ccb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3126,7 +3126,6 @@ dependencies = [ "clap", "libc", "nix 0.20.0", - "thiserror", "unicode-width", "utf-8", "uucore", diff --git a/src/uu/wc/Cargo.toml b/src/uu/wc/Cargo.toml index 1fdaa02f7..80d6014da 100644 --- a/src/uu/wc/Cargo.toml +++ b/src/uu/wc/Cargo.toml @@ -18,7 +18,6 @@ path = "src/wc.rs" clap = { version = "2.33", features = ["wrap_help"] } uucore = { version=">=0.0.9", package="uucore", path="../../uucore" } uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" } -thiserror = "1.0" bytecount = "0.6.2" utf-8 = "0.7.6" unicode-width = "0.1.8" diff --git a/src/uu/wc/src/count_fast.rs b/src/uu/wc/src/count_fast.rs index b23e9ed8f..f2081be8c 100644 --- a/src/uu/wc/src/count_fast.rs +++ b/src/uu/wc/src/count_fast.rs @@ -1,10 +1,10 @@ use crate::word_count::WordCount; -use super::{WcResult, WordCountable}; +use super::WordCountable; #[cfg(any(target_os = "linux", target_os = "android"))] use std::fs::{File, OpenOptions}; -use std::io::{ErrorKind, Read}; +use std::io::{self, ErrorKind, Read}; #[cfg(unix)] use libc::S_IFREG; @@ -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) -> WcResult { +pub(crate) fn count_bytes_fast(handle: &mut T) -> io::Result { let mut byte_count = 0; #[cfg(unix)] @@ -123,12 +123,12 @@ pub(crate) fn count_bytes_fast(handle: &mut T) -> WcResult continue, - Err(e) => return Err(e.into()), + Err(e) => return Err(e), } } } -pub(crate) fn count_bytes_and_lines_fast(handle: &mut R) -> WcResult { +pub(crate) fn count_bytes_and_lines_fast(handle: &mut R) -> io::Result { let mut total = WordCount::default(); let mut buf = [0; BUF_SIZE]; loop { @@ -139,7 +139,7 @@ pub(crate) fn count_bytes_and_lines_fast(handle: &mut R) -> WcResult continue, - Err(e) => return Err(e.into()), + Err(e) => return Err(e), } } } diff --git a/src/uu/wc/src/wc.rs b/src/uu/wc/src/wc.rs index 5832db25f..396f6eeaf 100644 --- a/src/uu/wc/src/wc.rs +++ b/src/uu/wc/src/wc.rs @@ -18,26 +18,15 @@ use utf8::{BufReadDecoder, BufReadDecoderError}; use word_count::{TitledWordCount, WordCount}; use clap::{crate_version, App, Arg, ArgMatches}; -use thiserror::Error; use std::cmp::max; use std::fs::{self, File}; -use std::io::{self, ErrorKind, Write}; +use std::io::{self, Write}; use std::path::{Path, PathBuf}; /// The minimum character width for formatting counts when reading from stdin. const MINIMUM_WIDTH: usize = 7; -#[derive(Error, Debug)] -pub enum WcError { - #[error("{0}")] - Io(#[from] io::Error), - #[error("Expected a file, found directory {0}")] - IsDirectory(PathBuf), -} - -type WcResult = Result; - struct Settings { show_bytes: bool, show_chars: bool, @@ -132,6 +121,13 @@ impl Input { Input::Stdin(StdinKind::Implicit) => None, } } + + fn path_display(&self) -> std::path::Display<'_> { + match self { + Input::Path(path) => path.display(), + Input::Stdin(_) => Path::display("'standard input'".as_ref()), + } + } } pub fn uumain(args: impl uucore::Args) -> i32 { @@ -206,8 +202,7 @@ pub fn uu_app() -> App<'static, 'static> { fn word_count_from_reader( mut reader: T, settings: &Settings, - path: &Path, -) -> WcResult { +) -> io::Result { let only_count_bytes = settings.show_bytes && (!(settings.show_chars || settings.show_lines @@ -273,7 +268,7 @@ fn word_count_from_reader( total.bytes += bytes.len(); } Err(BufReadDecoderError::Io(e)) => { - show_warning!("Error while reading {}: {}", path.display(), e); + return Err(e); } } } @@ -283,20 +278,16 @@ fn word_count_from_reader( Ok(total) } -fn word_count_from_input(input: &Input, settings: &Settings) -> WcResult { +fn word_count_from_input(input: &Input, settings: &Settings) -> io::Result { match input { Input::Stdin(_) => { let stdin = io::stdin(); let stdin_lock = stdin.lock(); - word_count_from_reader(stdin_lock, settings, "-".as_ref()) + word_count_from_reader(stdin_lock, settings) } Input::Path(path) => { - if path.is_dir() { - Err(WcError::IsDirectory(path.to_owned())) - } else { - let file = File::open(path)?; - word_count_from_reader(file, settings, path) - } + let file = File::open(path)?; + word_count_from_reader(file, settings) } } } @@ -310,18 +301,8 @@ fn word_count_from_input(input: &Input, settings: &Settings) -> WcResult { - show_error_custom_description!(path.display(), "Is a directory"); - } - (Input::Path(path), WcError::Io(e)) if e.kind() == ErrorKind::NotFound => { - show_error_custom_description!(path.display(), "No such file or directory"); - } - (_, e) => { - show_error!("{}", e); - } - }; +fn show_error(input: &Input, err: io::Error) { + show_error!("{}: {}", input.path_display(), err); } /// Compute the number of digits needed to represent any count for this input. @@ -343,7 +324,7 @@ fn show_error(input: &Input, err: WcError) { /// let input = Input::Stdin(StdinKind::Explicit); /// assert_eq!(7, digit_width(input)); /// ``` -fn digit_width(input: &Input) -> WcResult> { +fn digit_width(input: &Input) -> io::Result> { match input { Input::Stdin(_) => Ok(Some(MINIMUM_WIDTH)), Input::Path(filename) => { @@ -453,7 +434,7 @@ fn print_stats( settings: &Settings, result: &TitledWordCount, mut min_width: usize, -) -> WcResult<()> { +) -> io::Result<()> { let stdout = io::stdout(); let mut stdout_lock = stdout.lock(); diff --git a/tests/by-util/test_wc.rs b/tests/by-util/test_wc.rs index 1c6625ab4..8cf10656a 100644 --- a/tests/by-util/test_wc.rs +++ b/tests/by-util/test_wc.rs @@ -212,7 +212,7 @@ fn test_read_from_directory_error() { new_ucmd!() .args(&["."]) .fails() - .stderr_contains(".: Is a directory\n") + .stderr_contains(".: Is a directory") .stdout_is("0 0 0 .\n"); } @@ -222,5 +222,5 @@ fn test_read_from_nonexistent_file() { new_ucmd!() .args(&["bogusfile"]) .fails() - .stderr_contains("bogusfile: No such file or directory\n"); + .stderr_contains("bogusfile: No such file or directory"); }