Merge branch 'master' into chmod/compat

.vscode/cspell.dictionaries/workspace.wordlist.txt

@@ -9,6 +9,7 @@ aho-corasick
 backtrace
 blake2b_simd
 bstr
+bytecount
 byteorder
 chacha
 chrono
@@ -106,14 +107,18 @@ whoami
 
 # * vars/errno
 errno
+EACCES
 EBADF
+EBUSY
 EEXIST
 EINVAL
 ENODATA
 ENOENT
 ENOSYS
+ENOTEMPTY
 EOPNOTSUPP
 EPERM
+EROFS
 
 # * vars/fcntl
 F_GETFL

Cargo.lock

@@ -188,6 +188,12 @@ dependencies = [
  "utf8-width",
 ]
 
+[[package]]
+name = "bytecount"
+version = "0.6.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "72feb31ffc86498dacdbd0fcebb56138e7177a8cc5cea4516031d15ae85a742e"
+
 [[package]]
 name = "byteorder"
 version = "1.4.3"
@@ -2040,6 +2046,12 @@ dependencies = [
  "log",
 ]
 
+[[package]]
+name = "utf-8"
+version = "0.7.6"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "09cc8ee72d2a9becf2f2febe0205bbed8fc6615b7cb429ad062dc7b7ddd036a9"
+
 [[package]]
 name = "utf8-width"
 version = "0.1.5"
@@ -2790,6 +2802,7 @@ name = "uu_rmdir"
 version = "0.0.7"
 dependencies = [
  "clap",
+ "libc",
  "uucore",
  "uucore_procs",
 ]
@@ -3110,10 +3123,12 @@ dependencies = [
 name = "uu_wc"
 version = "0.0.7"
 dependencies = [
+ "bytecount",
  "clap",
  "libc",
  "nix 0.20.0",
- "thiserror",
+ "unicode-width",
+ "utf-8",
  "uucore",
  "uucore_procs",
 ]

DEVELOPER_INSTRUCTIONS.md

@@ -1,3 +1,29 @@
+Documentation
+-------------
+
+The source of the documentation is available on:
+
+https://uutils.github.io/coreutils-docs/coreutils/
+
+The documentation is updated everyday on this repository:
+
+https://github.com/uutils/coreutils-docs
+
+Running GNU tests
+-----------------
+
+<!-- spell-checker:ignore gnulib -->
+
+- Check out https://github.com/coreutils/coreutils next to your fork as gnu
+- Check out https://github.com/coreutils/gnulib next to your fork as gnulib
+- Rename the checkout of your fork to uutils
+
+At the end you should have uutils, gnu and gnulib checked out next to each other.
+
+- Run `cd uutils && ./util/build-gnu.sh && cd ..` to get everything ready (this may take a while)
+- Finally, you can run `tests with bash uutils/util/run-gnu-test.sh <test>`. Instead of `<test>` insert the test you want to run, e.g. `tests/misc/wc-proc`.
+
+
 Code Coverage Report Generation
 ---------------------------------
 

src/uu/base32/src/base_common.rs

@@ -122,7 +122,7 @@ pub fn base_app<'a>(name: &str, version: &'a str, about: &'a str) -> App<'static
 pub fn get_input<'a>(config: &Config, stdin_ref: &'a Stdin) -> Box<dyn Read + 'a> {
     match &config.to_read {
         Some(name) => {
-            let file_buf = safe_unwrap!(File::open(Path::new(name)));
+            let file_buf = crash_if_err!(1, File::open(Path::new(name)));
             Box::new(BufReader::new(file_buf)) // as Box<dyn Read>
         }
         None => {

src/uu/cp/src/cp.rs

@@ -231,8 +231,6 @@ fn usage() -> String {
 mod options {
     pub const ARCHIVE: &str = "archive";
     pub const ATTRIBUTES_ONLY: &str = "attributes-only";
-    pub const BACKUP: &str = "backup";
-    pub const BACKUP_NO_ARG: &str = "b";
     pub const CLI_SYMBOLIC_LINKS: &str = "cli-symbolic-links";
     pub const CONTEXT: &str = "context";
     pub const COPY_CONTENTS: &str = "copy-contents";
@@ -257,7 +255,6 @@ mod options {
     pub const REMOVE_DESTINATION: &str = "remove-destination";
     pub const SPARSE: &str = "sparse";
     pub const STRIP_TRAILING_SLASHES: &str = "strip-trailing-slashes";
-    pub const SUFFIX: &str = "suffix";
     pub const SYMBOLIC_LINK: &str = "symbolic-link";
     pub const TARGET_DIRECTORY: &str = "target-directory";
     pub const UPDATE: &str = "update";
@@ -355,24 +352,9 @@ pub fn uu_app() -> App<'static, 'static> {
              .conflicts_with(options::FORCE)
              .help("remove each existing destination file before attempting to open it \
                     (contrast with --force). On Windows, current only works for writeable files."))
-        .arg(Arg::with_name(options::BACKUP)
+        .arg(backup_control::arguments::backup())
-             .long(options::BACKUP)
+        .arg(backup_control::arguments::backup_no_args())
-             .help("make a backup of each existing destination file")
+        .arg(backup_control::arguments::suffix())
-             .takes_value(true)
-             .require_equals(true)
-             .min_values(0)
-             .value_name("CONTROL")
-        )
-        .arg(Arg::with_name(options::BACKUP_NO_ARG)
-             .short(options::BACKUP_NO_ARG)
-             .help("like --backup but does not accept an argument")
-        )
-        .arg(Arg::with_name(options::SUFFIX)
-             .short("S")
-             .long(options::SUFFIX)
-             .takes_value(true)
-             .value_name("SUFFIX")
-             .help("override the usual backup suffix"))
         .arg(Arg::with_name(options::UPDATE)
              .short("u")
              .long(options::UPDATE)
@@ -604,20 +586,12 @@ impl Options {
             || matches.is_present(options::RECURSIVE_ALIAS)
             || matches.is_present(options::ARCHIVE);
 
-        let backup_mode = backup_control::determine_backup_mode(
+        let backup_mode = match backup_control::determine_backup_mode(matches) {
-            matches.is_present(options::BACKUP_NO_ARG),
+            Err(e) => return Err(Error::Backup(format!("{}", e))),
-            matches.is_present(options::BACKUP),
-            matches.value_of(options::BACKUP),
-        );
-        let backup_mode = match backup_mode {
-            Err(err) => {
-                return Err(Error::Backup(err));
-            }
             Ok(mode) => mode,
         };
 
-        let backup_suffix =
+        let backup_suffix = backup_control::determine_backup_suffix(matches);
-            backup_control::determine_backup_suffix(matches.value_of(options::SUFFIX));
 
         let overwrite = OverwriteMode::from_matches(matches);
 

src/uu/csplit/src/csplit.rs

@@ -725,14 +725,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         .unwrap()
         .map(str::to_string)
         .collect();
-    let patterns = return_if_err!(1, patterns::get_patterns(&patterns[..]));
+    let patterns = crash_if_err!(1, patterns::get_patterns(&patterns[..]));
     let options = CsplitOptions::new(&matches);
     if file_name == "-" {
         let stdin = io::stdin();
         crash_if_err!(1, csplit(&options, patterns, stdin.lock()));
     } else {
-        let file = return_if_err!(1, File::open(file_name));
+        let file = crash_if_err!(1, File::open(file_name));
-        let file_metadata = return_if_err!(1, file.metadata());
+        let file_metadata = crash_if_err!(1, file.metadata());
         if !file_metadata.is_file() {
             crash!(1, "'{}' is not a regular file", file_name);
         }

src/uu/expand/src/expand.rs

@@ -329,12 +329,15 @@ fn expand(options: Options) {
                         // now dump out either spaces if we're expanding, or a literal tab if we're not
                         if init || !options.iflag {
                             if nts <= options.tspaces.len() {
-                                safe_unwrap!(output.write_all(options.tspaces[..nts].as_bytes()));
+                                crash_if_err!(
+                                    1,
+                                    output.write_all(options.tspaces[..nts].as_bytes())
+                                );
                             } else {
-                                safe_unwrap!(output.write_all(" ".repeat(nts).as_bytes()));
+                                crash_if_err!(1, output.write_all(" ".repeat(nts).as_bytes()));
                             };
                         } else {
-                            safe_unwrap!(output.write_all(&buf[byte..byte + nbytes]));
+                            crash_if_err!(1, output.write_all(&buf[byte..byte + nbytes]));
                         }
                     }
                     _ => {
@@ -352,14 +355,14 @@ fn expand(options: Options) {
                             init = false;
                         }
 
-                        safe_unwrap!(output.write_all(&buf[byte..byte + nbytes]));
+                        crash_if_err!(1, output.write_all(&buf[byte..byte + nbytes]));
                     }
                 }
 
                 byte += nbytes; // advance the pointer
             }
 
-            safe_unwrap!(output.flush());
+            crash_if_err!(1, output.flush());
             buf.truncate(0); // clear the buffer
         }
     }

src/uu/fold/src/fold.rs

@@ -119,7 +119,7 @@ fn fold(filenames: Vec<String>, bytes: bool, spaces: bool, width: usize) {
             stdin_buf = stdin();
             &mut stdin_buf as &mut dyn Read
         } else {
-            file_buf = safe_unwrap!(File::open(Path::new(filename)));
+            file_buf = crash_if_err!(1, File::open(Path::new(filename)));
             &mut file_buf as &mut dyn Read
         });
 

src/uu/hashsum/src/hashsum.rs

@@ -469,7 +469,7 @@ where
             stdin_buf = stdin();
             Box::new(stdin_buf) as Box<dyn Read>
         } else {
-            file_buf = safe_unwrap!(File::open(filename));
+            file_buf = crash_if_err!(1, File::open(filename));
             Box::new(file_buf) as Box<dyn Read>
         });
         if options.check {
@@ -486,19 +486,25 @@ where
             } else {
                 "+".to_string()
             };
-            let gnu_re = safe_unwrap!(Regex::new(&format!(
+            let gnu_re = crash_if_err!(
+                1,
+                Regex::new(&format!(
                     r"^(?P<digest>[a-fA-F0-9]{}) (?P<binary>[ \*])(?P<fileName>.*)",
                     modifier,
-            )));
+                ))
-            let bsd_re = safe_unwrap!(Regex::new(&format!(
+            );
+            let bsd_re = crash_if_err!(
+                1,
+                Regex::new(&format!(
                     r"^{algorithm} \((?P<fileName>.*)\) = (?P<digest>[a-fA-F0-9]{digest_size})",
                     algorithm = options.algoname,
                     digest_size = modifier,
-            )));
+                ))
+            );
 
             let buffer = file;
             for (i, line) in buffer.lines().enumerate() {
-                let line = safe_unwrap!(line);
+                let line = crash_if_err!(1, line);
                 let (ck_filename, sum, binary_check) = match gnu_re.captures(&line) {
                     Some(caps) => (
                         caps.name("fileName").unwrap().as_str(),
@@ -528,14 +534,17 @@ where
                         }
                     },
                 };
-                let f = safe_unwrap!(File::open(ck_filename));
+                let f = crash_if_err!(1, File::open(ck_filename));
                 let mut ckf = BufReader::new(Box::new(f) as Box<dyn Read>);
-                let real_sum = safe_unwrap!(digest_reader(
+                let real_sum = crash_if_err!(
+                    1,
+                    digest_reader(
                         &mut *options.digest,
                         &mut ckf,
                         binary_check,
                         options.output_bits
-                ))
+                    )
+                )
                 .to_ascii_lowercase();
                 if sum == real_sum {
                     if !options.quiet {
@@ -549,12 +558,15 @@ where
                 }
             }
         } else {
-            let sum = safe_unwrap!(digest_reader(
+            let sum = crash_if_err!(
+                1,
+                digest_reader(
                     &mut *options.digest,
                     &mut file,
                     options.binary,
                     options.output_bits
-            ));
+                )
+            );
             if options.tag {
                 println!("{} ({}) = {}", options.algoname, filename.display(), sum);
             } else {

src/uu/install/src/install.rs

@@ -155,8 +155,6 @@ static ABOUT: &str = "Copy SOURCE to DEST or multiple SOURCE(s) to the existing
  DIRECTORY, while setting permission modes and owner/group";
 
 static OPT_COMPARE: &str = "compare";
-static OPT_BACKUP: &str = "backup";
-static OPT_BACKUP_NO_ARG: &str = "backup2";
 static OPT_DIRECTORY: &str = "directory";
 static OPT_IGNORED: &str = "ignored";
 static OPT_CREATE_LEADING: &str = "create-leading";
@@ -166,7 +164,6 @@ static OPT_OWNER: &str = "owner";
 static OPT_PRESERVE_TIMESTAMPS: &str = "preserve-timestamps";
 static OPT_STRIP: &str = "strip";
 static OPT_STRIP_PROGRAM: &str = "strip-program";
-static OPT_SUFFIX: &str = "suffix";
 static OPT_TARGET_DIRECTORY: &str = "target-directory";
 static OPT_NO_TARGET_DIRECTORY: &str = "no-target-directory";
 static OPT_VERBOSE: &str = "verbose";
@@ -209,19 +206,10 @@ pub fn uu_app() -> App<'static, 'static> {
         .version(crate_version!())
         .about(ABOUT)
         .arg(
-                Arg::with_name(OPT_BACKUP)
+            backup_control::arguments::backup()
-                .long(OPT_BACKUP)
-                .help("make a backup of each existing destination file")
-                .takes_value(true)
-                .require_equals(true)
-                .min_values(0)
-                .value_name("CONTROL")
         )
         .arg(
-            // TODO implement flag
+            backup_control::arguments::backup_no_args()
-            Arg::with_name(OPT_BACKUP_NO_ARG)
-            .short("b")
-            .help("like --backup but does not accept an argument")
         )
         .arg(
             Arg::with_name(OPT_IGNORED)
@@ -290,14 +278,7 @@ pub fn uu_app() -> App<'static, 'static> {
                 .value_name("PROGRAM")
         )
         .arg(
-            // TODO implement flag
+            backup_control::arguments::suffix()
-            Arg::with_name(OPT_SUFFIX)
-                .short("S")
-                .long(OPT_SUFFIX)
-                .help("override the usual backup suffix")
-                .value_name("SUFFIX")
-                .takes_value(true)
-                .min_values(1)
         )
         .arg(
             // TODO implement flag
@@ -387,23 +368,14 @@ fn behavior(matches: &ArgMatches) -> UResult<Behavior> {
         None
     };
 
-    let backup_mode = backup_control::determine_backup_mode(
+    let backup_mode = backup_control::determine_backup_mode(matches)?;
-        matches.is_present(OPT_BACKUP_NO_ARG),
-        matches.is_present(OPT_BACKUP),
-        matches.value_of(OPT_BACKUP),
-    );
-    let backup_mode = match backup_mode {
-        Err(err) => return Err(USimpleError::new(1, err)),
-        Ok(mode) => mode,
-    };
-
     let target_dir = matches.value_of(OPT_TARGET_DIRECTORY).map(|d| d.to_owned());
 
     Ok(Behavior {
         main_function,
         specified_mode,
         backup_mode,
-        suffix: backup_control::determine_backup_suffix(matches.value_of(OPT_SUFFIX)),
+        suffix: backup_control::determine_backup_suffix(matches),
         owner: matches.value_of(OPT_OWNER).unwrap_or("").to_string(),
         group: matches.value_of(OPT_GROUP).unwrap_or("").to_string(),
         verbose: matches.is_present(OPT_VERBOSE),

src/uu/ln/src/ln.rs

@@ -54,7 +54,6 @@ enum LnError {
     FailedToLink(String),
     MissingDestination(String),
     ExtraOperand(String),
-    InvalidBackupMode(String),
 }
 
 impl Display for LnError {
@@ -72,7 +71,6 @@ impl Display for LnError {
                 s,
                 uucore::execution_phrase()
             ),
-            Self::InvalidBackupMode(s) => write!(f, "{}", s),
         }
     }
 }
@@ -87,7 +85,6 @@ impl UError for LnError {
             Self::FailedToLink(_) => 1,
             Self::MissingDestination(_) => 1,
             Self::ExtraOperand(_) => 1,
-            Self::InvalidBackupMode(_) => 1,
         }
     }
 }
@@ -119,13 +116,10 @@ fn long_usage() -> String {
 static ABOUT: &str = "change file owner and group";
 
 mod options {
-    pub const BACKUP_NO_ARG: &str = "b";
-    pub const BACKUP: &str = "backup";
     pub const FORCE: &str = "force";
     pub const INTERACTIVE: &str = "interactive";
     pub const NO_DEREFERENCE: &str = "no-dereference";
     pub const SYMBOLIC: &str = "symbolic";
-    pub const SUFFIX: &str = "suffix";
     pub const TARGET_DIRECTORY: &str = "target-directory";
     pub const NO_TARGET_DIRECTORY: &str = "no-target-directory";
     pub const RELATIVE: &str = "relative";
@@ -164,19 +158,8 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         OverwriteMode::NoClobber
     };
 
-    let backup_mode = backup_control::determine_backup_mode(
+    let backup_mode = backup_control::determine_backup_mode(&matches)?;
-        matches.is_present(options::BACKUP_NO_ARG),
+    let backup_suffix = backup_control::determine_backup_suffix(&matches);
-        matches.is_present(options::BACKUP),
-        matches.value_of(options::BACKUP),
-    );
-    let backup_mode = match backup_mode {
-        Err(err) => {
-            return Err(LnError::InvalidBackupMode(err).into());
-        }
-        Ok(mode) => mode,
-    };
-
-    let backup_suffix = backup_control::determine_backup_suffix(matches.value_of(options::SUFFIX));
 
     let settings = Settings {
         overwrite: overwrite_mode,
@@ -199,20 +182,8 @@ pub fn uu_app() -> App<'static, 'static> {
     App::new(uucore::util_name())
         .version(crate_version!())
         .about(ABOUT)
-        .arg(
+        .arg(backup_control::arguments::backup())
-            Arg::with_name(options::BACKUP)
+        .arg(backup_control::arguments::backup_no_args())
-                .long(options::BACKUP)
-                .help("make a backup of each existing destination file")
-                .takes_value(true)
-                .require_equals(true)
-                .min_values(0)
-                .value_name("CONTROL"),
-        )
-        .arg(
-            Arg::with_name(options::BACKUP_NO_ARG)
-                .short(options::BACKUP_NO_ARG)
-                .help("like --backup but does not accept an argument"),
-        )
         // TODO: opts.arg(
         //    Arg::with_name(("d", "directory", "allow users with appropriate privileges to attempt \
         //                                       to make hard links to directories");
@@ -250,14 +221,7 @@ pub fn uu_app() -> App<'static, 'static> {
                 // override added for https://github.com/uutils/coreutils/issues/2359
                 .overrides_with(options::SYMBOLIC),
         )
-        .arg(
+        .arg(backup_control::arguments::suffix())
-            Arg::with_name(options::SUFFIX)
-                .short("S")
-                .long(options::SUFFIX)
-                .help("override the usual backup suffix")
-                .value_name("SUFFIX")
-                .takes_value(true),
-        )
         .arg(
             Arg::with_name(options::TARGET_DIRECTORY)
                 .short("t")

src/uu/ls/src/ls.rs

@@ -1362,8 +1362,8 @@ fn enter_directory(dir: &PathData, config: &Config, out: &mut BufWriter<Stdout>)
         vec![]
     };
 
-    let mut temp: Vec<_> = safe_unwrap!(fs::read_dir(&dir.p_buf))
+    let mut temp: Vec<_> = crash_if_err!(1, fs::read_dir(&dir.p_buf))
-        .map(|res| safe_unwrap!(res))
+        .map(|res| crash_if_err!(1, res))
         .filter(|e| should_display(e, config))
         .map(|e| PathData::new(DirEntry::path(&e), Some(e.file_type()), None, config, false))
         .collect();

src/uu/ls/src/quoting_style.rs

@@ -1,6 +1,9 @@
 use std::char::from_digit;
 
-const SPECIAL_SHELL_CHARS: &str = "~`#$&*()|[]{};\\'\"<>?! ";
+// These are characters with special meaning in the shell (e.g. bash).
+// The first const contains characters that only have a special meaning when they appear at the beginning of a name.
+const SPECIAL_SHELL_CHARS_START: &[char] = &['~', '#'];
+const SPECIAL_SHELL_CHARS: &str = "`$&*()|[]{};\\'\"<>?! ";
 
 pub(super) enum QuotingStyle {
     Shell {
@@ -198,6 +201,8 @@ fn shell_without_escape(name: &str, quotes: Quotes, show_control_chars: bool) ->
             }
         }
     }
+
+    must_quote = must_quote || name.starts_with(SPECIAL_SHELL_CHARS_START);
     (escaped_str, must_quote)
 }
 
@@ -246,6 +251,7 @@ fn shell_with_escape(name: &str, quotes: Quotes) -> (String, bool) {
             }
         }
     }
+    must_quote = must_quote || name.starts_with(SPECIAL_SHELL_CHARS_START);
     (escaped_str, must_quote)
 }
 
@@ -659,4 +665,29 @@ mod tests {
             ],
         );
     }
+
+    #[test]
+    fn test_tilde_and_hash() {
+        check_names("~", vec![("'~'", "shell"), ("'~'", "shell-escape")]);
+        check_names(
+            "~name",
+            vec![("'~name'", "shell"), ("'~name'", "shell-escape")],
+        );
+        check_names(
+            "some~name",
+            vec![("some~name", "shell"), ("some~name", "shell-escape")],
+        );
+        check_names("name~", vec![("name~", "shell"), ("name~", "shell-escape")]);
+
+        check_names("#", vec![("'#'", "shell"), ("'#'", "shell-escape")]);
+        check_names(
+            "#name",
+            vec![("'#name'", "shell"), ("'#name'", "shell-escape")],
+        );
+        check_names(
+            "some#name",
+            vec![("some#name", "shell"), ("some#name", "shell-escape")],
+        );
+        check_names("name#", vec![("name#", "shell"), ("name#", "shell-escape")]);
+    }
 }

src/uu/mv/src/mv.rs

@@ -44,13 +44,10 @@ pub enum OverwriteMode {
 static ABOUT: &str = "Move SOURCE to DEST, or multiple SOURCE(s) to DIRECTORY.";
 static LONG_HELP: &str = "";
 
-static OPT_BACKUP: &str = "backup";
-static OPT_BACKUP_NO_ARG: &str = "b";
 static OPT_FORCE: &str = "force";
 static OPT_INTERACTIVE: &str = "interactive";
 static OPT_NO_CLOBBER: &str = "no-clobber";
 static OPT_STRIP_TRAILING_SLASHES: &str = "strip-trailing-slashes";
-static OPT_SUFFIX: &str = "suffix";
 static OPT_TARGET_DIRECTORY: &str = "target-directory";
 static OPT_NO_TARGET_DIRECTORY: &str = "no-target-directory";
 static OPT_UPDATE: &str = "update";
@@ -85,14 +82,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         .unwrap_or_default();
 
     let overwrite_mode = determine_overwrite_mode(&matches);
-    let backup_mode = backup_control::determine_backup_mode(
+    let backup_mode = match backup_control::determine_backup_mode(&matches) {
-        matches.is_present(OPT_BACKUP_NO_ARG),
+        Err(e) => {
-        matches.is_present(OPT_BACKUP),
+            show!(e);
-        matches.value_of(OPT_BACKUP),
-    );
-    let backup_mode = match backup_mode {
-        Err(err) => {
-            show_usage_error!("{}", err);
             return 1;
         }
         Ok(mode) => mode,
@@ -103,7 +95,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         return 1;
     }
 
-    let backup_suffix = backup_control::determine_backup_suffix(matches.value_of(OPT_SUFFIX));
+    let backup_suffix = backup_control::determine_backup_suffix(&matches);
 
     let behavior = Behavior {
         overwrite: overwrite_mode,
@@ -137,18 +129,10 @@ pub fn uu_app() -> App<'static, 'static> {
         .version(crate_version!())
         .about(ABOUT)
     .arg(
-            Arg::with_name(OPT_BACKUP)
+        backup_control::arguments::backup()
-            .long(OPT_BACKUP)
-            .help("make a backup of each existing destination file")
-            .takes_value(true)
-            .require_equals(true)
-            .min_values(0)
-            .value_name("CONTROL")
     )
     .arg(
-            Arg::with_name(OPT_BACKUP_NO_ARG)
+        backup_control::arguments::backup_no_args()
-            .short(OPT_BACKUP_NO_ARG)
-            .help("like --backup but does not accept an argument")
     )
     .arg(
             Arg::with_name(OPT_FORCE)
@@ -173,12 +157,7 @@ pub fn uu_app() -> App<'static, 'static> {
             .help("remove any trailing slashes from each SOURCE argument")
     )
     .arg(
-            Arg::with_name(OPT_SUFFIX)
+        backup_control::arguments::suffix()
-            .short("S")
-            .long(OPT_SUFFIX)
-            .help("override the usual backup suffix")
-            .takes_value(true)
-            .value_name("SUFFIX")
     )
     .arg(
         Arg::with_name(OPT_TARGET_DIRECTORY)

src/uu/pinky/src/pinky.rs

@@ -291,7 +291,7 @@ impl Pinky {
 
         let mut s = ut.host();
         if self.include_where && !s.is_empty() {
-            s = safe_unwrap!(ut.canon_host());
+            s = crash_if_err!(1, ut.canon_host());
             print!(" {}", s);
         }
 

src/uu/rmdir/Cargo.toml

@@ -18,6 +18,7 @@ path = "src/rmdir.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" }
+libc = "0.2.42"
 
 [[bin]]
 name = "rmdir"

src/uu/rmdir/src/rmdir.rs

@@ -11,8 +11,11 @@
 extern crate uucore;
 
 use clap::{crate_version, App, Arg};
-use std::fs;
+use std::fs::{read_dir, remove_dir};
+use std::io;
 use std::path::Path;
+use uucore::error::{set_exit_code, strip_errno, UResult};
+use uucore::util_name;
 
 static ABOUT: &str = "Remove the DIRECTORY(ies), if they are empty.";
 static OPT_IGNORE_FAIL_NON_EMPTY: &str = "ignore-fail-on-non-empty";
@@ -21,35 +24,158 @@ static OPT_VERBOSE: &str = "verbose";
 
 static ARG_DIRS: &str = "dirs";
 
-#[cfg(unix)]
-static ENOTDIR: i32 = 20;
-#[cfg(windows)]
-static ENOTDIR: i32 = 267;
-
 fn usage() -> String {
     format!("{0} [OPTION]... DIRECTORY...", uucore::execution_phrase())
 }
 
-pub fn uumain(args: impl uucore::Args) -> i32 {
+#[uucore_procs::gen_uumain]
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let usage = usage();
 
     let matches = uu_app().usage(&usage[..]).get_matches_from(args);
 
-    let dirs: Vec<String> = matches
+    let opts = Opts {
-        .values_of(ARG_DIRS)
+        ignore: matches.is_present(OPT_IGNORE_FAIL_NON_EMPTY),
-        .map(|v| v.map(ToString::to_string).collect())
+        parents: matches.is_present(OPT_PARENTS),
-        .unwrap_or_default();
+        verbose: matches.is_present(OPT_VERBOSE),
+    };
 
-    let ignore = matches.is_present(OPT_IGNORE_FAIL_NON_EMPTY);
+    for path in matches
-    let parents = matches.is_present(OPT_PARENTS);
+        .values_of_os(ARG_DIRS)
-    let verbose = matches.is_present(OPT_VERBOSE);
+        .unwrap_or_default()
+        .map(Path::new)
+    {
+        if let Err(error) = remove(path, opts) {
+            let Error { error, path } = error;
 
-    match remove(dirs, ignore, parents, verbose) {
+            if opts.ignore && dir_not_empty(&error, path) {
-        Ok(()) => ( /* pass */ ),
+                continue;
-        Err(e) => return e,
             }
 
-    0
+            set_exit_code(1);
+
+            // If `foo` is a symlink to a directory then `rmdir foo/` may give
+            // a "not a directory" error. This is confusing as `rm foo/` says
+            // "is a directory".
+            // This differs from system to system. Some don't give an error.
+            // Windows simply allows calling RemoveDirectory on symlinks so we
+            // don't need to worry about it here.
+            // GNU rmdir seems to print "Symbolic link not followed" if:
+            // - It has a trailing slash
+            // - It's a symlink
+            // - It either points to a directory or dangles
+            #[cfg(unix)]
+            {
+                use std::ffi::OsStr;
+                use std::os::unix::ffi::OsStrExt;
+
+                fn is_symlink(path: &Path) -> io::Result<bool> {
+                    Ok(path.symlink_metadata()?.file_type().is_symlink())
+                }
+
+                fn points_to_directory(path: &Path) -> io::Result<bool> {
+                    Ok(path.metadata()?.file_type().is_dir())
+                }
+
+                let path = path.as_os_str().as_bytes();
+                if error.raw_os_error() == Some(libc::ENOTDIR) && path.ends_with(b"/") {
+                    // Strip the trailing slash or .symlink_metadata() will follow the symlink
+                    let path: &Path = OsStr::from_bytes(&path[..path.len() - 1]).as_ref();
+                    if is_symlink(path).unwrap_or(false)
+                        && points_to_directory(path).unwrap_or(true)
+                    {
+                        show_error!(
+                            "failed to remove '{}/': Symbolic link not followed",
+                            path.display()
+                        );
+                        continue;
+                    }
+                }
+            }
+
+            show_error!(
+                "failed to remove '{}': {}",
+                path.display(),
+                strip_errno(&error)
+            );
+        }
+    }
+
+    Ok(())
+}
+
+struct Error<'a> {
+    error: io::Error,
+    path: &'a Path,
+}
+
+fn remove(mut path: &Path, opts: Opts) -> Result<(), Error<'_>> {
+    remove_single(path, opts)?;
+    if opts.parents {
+        while let Some(new) = path.parent() {
+            path = new;
+            if path.as_os_str() == "" {
+                break;
+            }
+            remove_single(path, opts)?;
+        }
+    }
+    Ok(())
+}
+
+fn remove_single(path: &Path, opts: Opts) -> Result<(), Error<'_>> {
+    if opts.verbose {
+        println!("{}: removing directory, '{}'", util_name(), path.display());
+    }
+    remove_dir(path).map_err(|error| Error { error, path })
+}
+
+// POSIX: https://pubs.opengroup.org/onlinepubs/009696799/functions/rmdir.html
+#[cfg(not(windows))]
+const NOT_EMPTY_CODES: &[i32] = &[libc::ENOTEMPTY, libc::EEXIST];
+
+// 145 is ERROR_DIR_NOT_EMPTY, determined experimentally.
+#[cfg(windows)]
+const NOT_EMPTY_CODES: &[i32] = &[145];
+
+// Other error codes you might get for directories that could be found and are
+// not empty.
+// This is a subset of the error codes listed in rmdir(2) from the Linux man-pages
+// project. Maybe other systems have additional codes that apply?
+#[cfg(not(windows))]
+const PERHAPS_EMPTY_CODES: &[i32] = &[libc::EACCES, libc::EBUSY, libc::EPERM, libc::EROFS];
+
+// Probably incomplete, I can't find a list of possible errors for
+// RemoveDirectory anywhere.
+#[cfg(windows)]
+const PERHAPS_EMPTY_CODES: &[i32] = &[
+    5, // ERROR_ACCESS_DENIED, found experimentally.
+];
+
+fn dir_not_empty(error: &io::Error, path: &Path) -> bool {
+    if let Some(code) = error.raw_os_error() {
+        if NOT_EMPTY_CODES.contains(&code) {
+            return true;
+        }
+        // If --ignore-fail-on-non-empty is used then we want to ignore all errors
+        // for non-empty directories, even if the error was e.g. because there's
+        // no permission. So we do an additional check.
+        if PERHAPS_EMPTY_CODES.contains(&code) {
+            if let Ok(mut iterator) = read_dir(path) {
+                if iterator.next().is_some() {
+                    return true;
+                }
+            }
+        }
+    }
+    false
+}
+
+#[derive(Clone, Copy, Debug)]
+struct Opts {
+    ignore: bool,
+    parents: bool,
+    verbose: bool,
 }
 
 pub fn uu_app() -> App<'static, 'static> {
@@ -84,57 +210,3 @@ pub fn uu_app() -> App<'static, 'static> {
                 .required(true),
         )
 }
-
-fn remove(dirs: Vec<String>, ignore: bool, parents: bool, verbose: bool) -> Result<(), i32> {
-    let mut r = Ok(());
-
-    for dir in &dirs {
-        let path = Path::new(&dir[..]);
-        r = remove_dir(path, ignore, verbose).and(r);
-        if parents {
-            let mut p = path;
-            while let Some(new_p) = p.parent() {
-                p = new_p;
-                match p.as_os_str().to_str() {
-                    None => break,
-                    Some(s) => match s {
-                        "" | "." | "/" => break,
-                        _ => (),
-                    },
-                };
-                r = remove_dir(p, ignore, verbose).and(r);
-            }
-        }
-    }
-
-    r
-}
-
-fn remove_dir(path: &Path, ignore: bool, verbose: bool) -> Result<(), i32> {
-    let mut read_dir = fs::read_dir(path).map_err(|e| {
-        if e.raw_os_error() == Some(ENOTDIR) {
-            show_error!("failed to remove '{}': Not a directory", path.display());
-        } else {
-            show_error!("reading directory '{}': {}", path.display(), e);
-        }
-        1
-    })?;
-
-    let mut r = Ok(());
-
-    if read_dir.next().is_none() {
-        match fs::remove_dir(path) {
-            Err(e) => {
-                show_error!("removing directory '{}': {}", path.display(), e);
-                r = Err(1);
-            }
-            Ok(_) if verbose => println!("removing directory, '{}'", path.display()),
-            _ => (),
-        }
-    } else if !ignore {
-        show_error!("failed to remove '{}': Directory not empty", path.display());
-        r = Err(1);
-    }
-
-    r
-}

src/uu/seq/src/seq.rs

@@ -105,7 +105,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         let dec = slice.find('.').unwrap_or(len);
         largest_dec = len - dec;
         padding = dec;
-        return_if_err!(1, slice.parse())
+        crash_if_err!(1, slice.parse())
     } else {
         Number::BigInt(BigInt::one())
     };
@@ -115,7 +115,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         let dec = slice.find('.').unwrap_or(len);
         largest_dec = cmp::max(largest_dec, len - dec);
         padding = cmp::max(padding, dec);
-        return_if_err!(1, slice.parse())
+        crash_if_err!(1, slice.parse())
     } else {
         Number::BigInt(BigInt::one())
     };
@@ -130,7 +130,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     let last = {
         let slice = numbers[numbers.len() - 1];
         padding = cmp::max(padding, slice.find('.').unwrap_or_else(|| slice.len()));
-        return_if_err!(1, slice.parse())
+        crash_if_err!(1, slice.parse())
     };
     if largest_dec > 0 {
         largest_dec -= 1;

src/uu/sort/src/sort.rs

@@ -45,7 +45,7 @@ use std::path::Path;
 use std::path::PathBuf;
 use std::str::Utf8Error;
 use unicode_width::UnicodeWidthStr;
-use uucore::error::{set_exit_code, UError, UResult, USimpleError, UUsageError};
+use uucore::error::{set_exit_code, strip_errno, UError, UResult, USimpleError, UUsageError};
 use uucore::parse_size::{parse_size, ParseSizeError};
 use uucore::version_cmp::version_cmp;
 use uucore::InvalidEncodingHandling;
@@ -197,27 +197,17 @@ impl Display for SortError {
                     Ok(())
                 }
             }
-            SortError::OpenFailed { path, error } => write!(
+            SortError::OpenFailed { path, error } => {
-                f,
+                write!(f, "open failed: {}: {}", path, strip_errno(error))
-                "open failed: {}: {}",
+            }
-                path,
-                strip_errno(&error.to_string())
-            ),
             SortError::ParseKeyError { key, msg } => {
                 write!(f, "failed to parse key `{}`: {}", key, msg)
             }
-            SortError::ReadFailed { path, error } => write!(
+            SortError::ReadFailed { path, error } => {
-                f,
+                write!(f, "cannot read: {}: {}", path, strip_errno(error))
-                "cannot read: {}: {}",
+            }
-                path,
-                strip_errno(&error.to_string())
-            ),
             SortError::OpenTmpFileFailed { error } => {
-                write!(
+                write!(f, "failed to open temporary file: {}", strip_errno(error))
-                    f,
-                    "failed to open temporary file: {}",
-                    strip_errno(&error.to_string())
-                )
             }
             SortError::CompressProgExecutionFailed { code } => {
                 write!(f, "couldn't execute compress program: errno {}", code)
@@ -1814,11 +1804,6 @@ fn print_sorted<'a, T: Iterator<Item = &'a Line<'a>>>(
     }
 }
 
-/// Strips the trailing " (os error XX)" from io error strings.
-fn strip_errno(err: &str) -> &str {
-    &err[..err.find(" (os error ").unwrap_or(err.len())]
-}
-
 fn open(path: impl AsRef<OsStr>) -> UResult<Box<dyn Read + Send>> {
     let path = path.as_ref();
     if path == "-" {

src/uu/stdbuf/src/stdbuf.rs

@@ -170,7 +170,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     let command_params: Vec<&str> = command_values.collect();
 
     let mut tmp_dir = tempdir().unwrap();
-    let (preload_env, libstdbuf) = return_if_err!(1, get_preload_env(&mut tmp_dir));
+    let (preload_env, libstdbuf) = crash_if_err!(1, get_preload_env(&mut tmp_dir));
     command.env(preload_env, libstdbuf);
     set_command_env(&mut command, "_STDBUF_I", options.stdin);
     set_command_env(&mut command, "_STDBUF_O", options.stdout);

src/uu/timeout/src/timeout.rs

@@ -221,7 +221,7 @@ fn timeout(
                     cmd[0]
                 );
             }
-            return_if_err!(ERR_EXIT_STATUS, process.send_signal(signal));
+            crash_if_err!(ERR_EXIT_STATUS, process.send_signal(signal));
             if let Some(kill_after) = kill_after {
                 match process.wait_or_timeout(kill_after) {
                     Ok(Some(status)) => {
@@ -235,13 +235,13 @@ fn timeout(
                         if verbose {
                             show_error!("sending signal KILL to command '{}'", cmd[0]);
                         }
-                        return_if_err!(
+                        crash_if_err!(
                             ERR_EXIT_STATUS,
                             process.send_signal(
                                 uucore::signals::signal_by_name_or_value("KILL").unwrap()
                             )
                         );
-                        return_if_err!(ERR_EXIT_STATUS, process.wait());
+                        crash_if_err!(ERR_EXIT_STATUS, process.wait());
                         137
                     }
                     Err(_) => 124,
@@ -251,7 +251,7 @@ fn timeout(
             }
         }
         Err(_) => {
-            return_if_err!(ERR_EXIT_STATUS, process.send_signal(signal));
+            crash_if_err!(ERR_EXIT_STATUS, process.send_signal(signal));
             ERR_EXIT_STATUS
         }
     }

src/uu/uname/src/uname.rs

@@ -53,7 +53,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     let usage = format!("{} [OPTION]...", uucore::execution_phrase());
     let matches = uu_app().usage(&usage[..]).get_matches_from(args);
 
-    let uname = return_if_err!(1, PlatformInfo::new());
+    let uname = crash_if_err!(1, PlatformInfo::new());
     let mut output = String::new();
 
     let all = matches.is_present(options::ALL);

src/uu/unexpand/src/unexpand.rs

@@ -178,13 +178,13 @@ fn write_tabs(
                 break;
             }
 
-            safe_unwrap!(output.write_all(b"\t"));
+            crash_if_err!(1, output.write_all(b"\t"));
             scol += nts;
         }
     }
 
     while col > scol {
-        safe_unwrap!(output.write_all(b" "));
+        crash_if_err!(1, output.write_all(b" "));
         scol += 1;
     }
 }
@@ -272,7 +272,7 @@ fn unexpand(options: Options) {
                         init,
                         true,
                     );
-                    safe_unwrap!(output.write_all(&buf[byte..]));
+                    crash_if_err!(1, output.write_all(&buf[byte..]));
                     scol = col;
                     break;
                 }
@@ -292,7 +292,7 @@ fn unexpand(options: Options) {
                         };
 
                         if !tabs_buffered {
-                            safe_unwrap!(output.write_all(&buf[byte..byte + nbytes]));
+                            crash_if_err!(1, output.write_all(&buf[byte..byte + nbytes]));
                             scol = col; // now printed up to this column
                         }
                     }
@@ -317,7 +317,7 @@ fn unexpand(options: Options) {
                         } else {
                             0
                         };
-                        safe_unwrap!(output.write_all(&buf[byte..byte + nbytes]));
+                        crash_if_err!(1, output.write_all(&buf[byte..byte + nbytes]));
                         scol = col; // we've now printed up to this column
                     }
                 }
@@ -336,7 +336,7 @@ fn unexpand(options: Options) {
                 init,
                 true,
             );
-            safe_unwrap!(output.flush());
+            crash_if_err!(1, output.flush());
             buf.truncate(0); // clear out the buffer
         }
     }

src/uu/wc/BENCHMARKING.md

@@ -0,0 +1,124 @@
+# Benchmarking wc
+
+<!-- spell-checker:ignore (words) uuwc uucat largefile somefile Mshortlines moby lwcm cmds tablefmt -->
+
+Much of what makes wc fast is avoiding unnecessary work. It has multiple strategies, depending on which data is requested.
+
+## Strategies
+
+### Counting bytes
+
+In the case of `wc -c` the content of the input doesn't have to be inspected at all, only the size has to be known. That enables a few optimizations.
+
+#### File size
+
+If it can, wc reads the file size directly. This is not interesting to benchmark, except to see if it still works. Try `wc -c largefile`.
+
+#### `splice()`
+
+On Linux `splice()` is used to get the input's length while discarding it directly.
+
+The best way I've found to generate a fast input to test `splice()` is to pipe the output of uutils `cat` into it. Note that GNU `cat` is slower and therefore less suitable, and that if a file is given as its input directly (as in `wc -c < largefile`) the first strategy kicks in. Try `uucat somefile | wc -c`.
+
+### Counting lines
+
+In the case of `wc -l` or `wc -cl` the input doesn't have to be decoded. It's read in chunks and the `bytecount` crate is used to count the newlines.
+
+It's useful to vary the line length in the input. GNU wc seems particularly bad at short lines.
+
+### Processing unicode
+
+This is the most general strategy, and it's necessary for counting words, characters, and line lengths. Individual steps are still switched on and off depending on what must be reported.
+
+Try varying which of the `-w`, `-m`, `-l` and `-L` flags are used. (The `-c` flag is unlikely to make a difference.)
+
+Passing no flags is equivalent to passing `-wcl`. That case should perhaps be given special attention as it's the default.
+
+## Generating files
+
+To generate a file with many very short lines, run `yes | head -c50000000 > 25Mshortlines`.
+
+To get a file with less artificial contents, download a book from Project Gutenberg and concatenate it a lot of times:
+
+```
+wget https://www.gutenberg.org/files/2701/2701-0.txt -O moby.txt
+cat moby.txt moby.txt moby.txt moby.txt > moby4.txt
+cat moby4.txt moby4.txt moby4.txt moby4.txt > moby16.txt
+cat moby16.txt moby16.txt moby16.txt moby16.txt > moby64.txt
+```
+
+And get one with lots of unicode too:
+
+```
+wget https://www.gutenberg.org/files/30613/30613-0.txt -O odyssey.txt
+cat odyssey.txt odyssey.txt odyssey.txt odyssey.txt > odyssey4.txt
+cat odyssey4.txt odyssey4.txt odyssey4.txt odyssey4.txt > odyssey16.txt
+cat odyssey16.txt odyssey16.txt odyssey16.txt odyssey16.txt > odyssey64.txt
+cat odyssey64.txt odyssey64.txt odyssey64.txt odyssey64.txt > odyssey256.txt
+```
+
+Finally, it's interesting to try a binary file. Look for one with `du -sh /usr/bin/* | sort -h`. On my system `/usr/bin/docker` is a good candidate as it's fairly large.
+
+## Running benchmarks
+
+Use [`hyperfine`](https://github.com/sharkdp/hyperfine) to compare the performance. For example, `hyperfine 'wc somefile' 'uuwc somefile'`.
+
+If you want to get fancy and exhaustive, generate a table:
+
+|                        |   moby64.txt |   odyssey256.txt |   25Mshortlines |   /usr/bin/docker |
+|------------------------|--------------|------------------|-----------------|-------------------|
+| `wc <FILE>`            |       1.3965 |           1.6182 |          5.2967 |            2.2294 |
+| `wc -c <FILE>`         |       0.8134 |           1.2774 |          0.7732 |            0.9106 |
+| `uucat <FILE> | wc -c` |       2.7760 |           2.5565 |          2.3769 |            2.3982 |
+| `wc -l <FILE>`         |       1.1441 |           1.2854 |          2.9681 |            1.1493 |
+| `wc -L <FILE>`         |       2.1087 |           1.2551 |          5.4577 |            2.1490 |
+| `wc -m <FILE>`         |       2.7272 |           2.1704 |          7.3371 |            3.4347 |
+| `wc -w <FILE>`         |       1.9007 |           1.5206 |          4.7851 |            2.8529 |
+| `wc -lwcmL <FILE>`     |       1.1687 |           0.9169 |          4.4092 |            2.0663 |
+
+Beware that:
+- Results are fuzzy and change from run to run
+- You'll often want to check versions of uutils wc against each other instead of against GNU
+- This takes a lot of time to generate
+- This only shows the relative speedup, not the absolute time, which may be misleading if the time is very short
+
+Created by the following Python script:
+```python
+import json
+import subprocess
+
+from tabulate import tabulate
+
+bins = ["wc", "uuwc"]
+files = ["moby64.txt", "odyssey256.txt", "25Mshortlines", "/usr/bin/docker"]
+cmds = [
+    "{cmd} {file}",
+    "{cmd} -c {file}",
+    "uucat {file} | {cmd} -c",
+    "{cmd} -l {file}",
+    "{cmd} -L {file}",
+    "{cmd} -m {file}",
+    "{cmd} -w {file}",
+    "{cmd} -lwcmL {file}",
+]
+
+table = []
+for cmd in cmds:
+    row = ["`" + cmd.format(cmd="wc", file="<FILE>") + "`"]
+    for file in files:
+        subprocess.run(
+            [
+                "hyperfine",
+                cmd.format(cmd=bins[0], file=file),
+                cmd.format(cmd=bins[1], file=file),
+                "--export-json=out.json",
+            ],
+            check=True,
+        )
+        with open("out.json") as f:
+            res = json.load(f)["results"]
+        row.append(round(res[0]["mean"] / res[1]["mean"], 4))
+    table.append(row)
+print(tabulate(table, [""] + files, tablefmt="github"))
+```
+(You may have to adjust the `bins` and `files` variables depending on your setup, and please do add other interesting cases to `cmds`.)

src/uu/wc/Cargo.toml

@@ -18,7 +18,9 @@ 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"
 
 [target.'cfg(unix)'.dependencies]
 nix = "0.20"

src/uu/wc/src/count_fast.rs

@@ -1,13 +1,15 @@
-use super::{WcResult, WordCountable};
+use crate::word_count::WordCount;
+
+use super::WordCountable;
 
 #[cfg(any(target_os = "linux", target_os = "android"))]
 use std::fs::{File, OpenOptions};
-use std::io::ErrorKind;
+use std::io::{self, ErrorKind, Read};
 
 #[cfg(unix)]
 use libc::S_IFREG;
 #[cfg(unix)]
-use nix::sys::stat::fstat;
+use nix::sys::stat;
 #[cfg(any(target_os = "linux", target_os = "android"))]
 use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
 
@@ -18,7 +20,9 @@ use nix::fcntl::{splice, SpliceFFlags};
 #[cfg(any(target_os = "linux", target_os = "android"))]
 use nix::unistd::pipe;
 
-const BUF_SIZE: usize = 16384;
+const BUF_SIZE: usize = 16 * 1024;
+#[cfg(any(target_os = "linux", target_os = "android"))]
+const SPLICE_SIZE: usize = 128 * 1024;
 
 /// Splice wrapper which handles short writes
 #[cfg(any(target_os = "linux", target_os = "android"))]
@@ -37,15 +41,24 @@ fn splice_exact(read_fd: RawFd, write_fd: RawFd, num_bytes: usize) -> nix::Resul
 
 /// This is a Linux-specific function to count the number of bytes using the
 /// `splice` system call, which is faster than using `read`.
+///
+/// On error it returns the number of bytes it did manage to read, since the
+/// caller will fall back to a simpler method.
 #[inline]
 #[cfg(any(target_os = "linux", target_os = "android"))]
-fn count_bytes_using_splice(fd: RawFd) -> nix::Result<usize> {
+fn count_bytes_using_splice(fd: RawFd) -> Result<usize, usize> {
     let null_file = OpenOptions::new()
         .write(true)
         .open("/dev/null")
-        .map_err(|_| nix::Error::last())?;
+        .map_err(|_| 0_usize)?;
     let null = null_file.as_raw_fd();
-    let (pipe_rd, pipe_wr) = pipe()?;
+    let null_rdev = stat::fstat(null).map_err(|_| 0_usize)?.st_rdev;
+    if (stat::major(null_rdev), stat::minor(null_rdev)) != (1, 3) {
+        // This is not a proper /dev/null, writing to it is probably bad
+        // Bit of an edge case, but it has been known to happen
+        return Err(0);
+    }
+    let (pipe_rd, pipe_wr) = pipe().map_err(|_| 0_usize)?;
 
     // Ensure the pipe is closed when the function returns.
     // SAFETY: The file descriptors do not have other owners.
@@ -53,12 +66,16 @@ fn count_bytes_using_splice(fd: RawFd) -> nix::Result<usize> {
 
     let mut byte_count = 0;
     loop {
-        let res = splice(fd, None, pipe_wr, None, BUF_SIZE, SpliceFFlags::empty())?;
+        match splice(fd, None, pipe_wr, None, SPLICE_SIZE, SpliceFFlags::empty()) {
-        if res == 0 {
+            Ok(0) => break,
-            break;
+            Ok(res) => {
-        }
                 byte_count += res;
-        splice_exact(pipe_rd, null, res)?;
+                if splice_exact(pipe_rd, null, res).is_err() {
+                    return Err(byte_count);
+                }
+            }
+            Err(_) => return Err(byte_count),
+        };
     }
 
     Ok(byte_count)
@@ -72,23 +89,26 @@ fn count_bytes_using_splice(fd: RawFd) -> nix::Result<usize> {
 ///   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<T: WordCountable>(handle: &mut T) -> WcResult<usize> {
+pub(crate) fn count_bytes_fast<T: WordCountable>(handle: &mut T) -> (usize, Option<io::Error>) {
+    let mut byte_count = 0;
+
     #[cfg(unix)]
     {
         let fd = handle.as_raw_fd();
-        if let Ok(stat) = fstat(fd) {
+        if let Ok(stat) = stat::fstat(fd) {
             // If the file is regular, then the `st_size` should hold
             // the file's size in bytes.
             if (stat.st_mode & S_IFREG) != 0 {
-                return Ok(stat.st_size as usize);
+                return (stat.st_size as usize, None);
             }
             #[cfg(any(target_os = "linux", target_os = "android"))]
             {
                 // Else, if we're on Linux and our file is a FIFO pipe
                 // (or stdin), we use splice to count the number of bytes.
                 if (stat.st_mode & S_IFIFO) != 0 {
-                    if let Ok(n) = count_bytes_using_splice(fd) {
+                    match count_bytes_using_splice(fd) {
-                        return Ok(n);
+                        Ok(n) => return (n, None),
+                        Err(n) => byte_count = n,
                     }
                 }
             }
@@ -97,15 +117,32 @@ pub(crate) fn count_bytes_fast<T: WordCountable>(handle: &mut T) -> WcResult<usi
 
     // Fall back on `read`, but without the overhead of counting words and lines.
     let mut buf = [0_u8; BUF_SIZE];
-    let mut byte_count = 0;
     loop {
         match handle.read(&mut buf) {
-            Ok(0) => 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.into()),
+            Err(e) => return (byte_count, Some(e)),
+        }
+    }
+}
+
+pub(crate) fn count_bytes_and_lines_fast<R: Read>(
+    handle: &mut R,
+) -> (WordCount, Option<io::Error>) {
+    let mut total = WordCount::default();
+    let mut buf = [0; BUF_SIZE];
+    loop {
+        match handle.read(&mut buf) {
+            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 (total, Some(e)),
         }
     }
 }

src/uu/wc/src/countable.rs

@@ -4,7 +4,7 @@
 //! for some common file-like objects. Use the [`WordCountable::lines`]
 //! method to get an iterator over lines of a file-like object.
 use std::fs::File;
-use std::io::{self, BufRead, BufReader, Read, StdinLock};
+use std::io::{BufRead, BufReader, Read, StdinLock};
 
 #[cfg(unix)]
 use std::os::unix::io::AsRawFd;
@@ -12,61 +12,26 @@ use std::os::unix::io::AsRawFd;
 #[cfg(unix)]
 pub trait WordCountable: AsRawFd + Read {
     type Buffered: BufRead;
-    fn lines(self) -> Lines<Self::Buffered>;
+    fn buffered(self) -> Self::Buffered;
 }
 
 #[cfg(not(unix))]
 pub trait WordCountable: Read {
     type Buffered: BufRead;
-    fn lines(self) -> Lines<Self::Buffered>;
+    fn buffered(self) -> Self::Buffered;
 }
 
 impl WordCountable for StdinLock<'_> {
     type Buffered = Self;
 
-    fn lines(self) -> Lines<Self::Buffered>
+    fn buffered(self) -> Self::Buffered {
-    where
+        self
-        Self: Sized,
-    {
-        Lines { buf: self }
     }
 }
 impl WordCountable for File {
     type Buffered = BufReader<Self>;
 
-    fn lines(self) -> Lines<Self::Buffered>
+    fn buffered(self) -> Self::Buffered {
-    where
+        BufReader::new(self)
-        Self: Sized,
-    {
-        Lines {
-            buf: BufReader::new(self),
-        }
-    }
-}
-
-/// An iterator over the lines of an instance of `BufRead`.
-///
-/// Similar to [`io::Lines`] but yields each line as a `Vec<u8>` and
-/// includes the newline character (`\n`, the `0xA` byte) that
-/// terminates the line.
-///
-/// [`io::Lines`]:: io::Lines
-pub struct Lines<B> {
-    buf: B,
-}
-
-impl<B: BufRead> Iterator for Lines<B> {
-    type Item = io::Result<Vec<u8>>;
-
-    fn next(&mut self) -> Option<Self::Item> {
-        let mut line = Vec::new();
-
-        // reading from a TTY seems to raise a condition on, rather than return Some(0) like a file.
-        // hence the option wrapped in a result here
-        match self.buf.read_until(b'\n', &mut line) {
-            Ok(0) => None,
-            Ok(_n) => Some(Ok(line)),
-            Err(e) => Some(Err(e)),
-        }
     }
 }

src/uu/wc/src/wc.rs

@@ -8,33 +8,25 @@
 #[macro_use]
 extern crate uucore;
 
-mod count_bytes;
+mod count_fast;
 mod countable;
 mod word_count;
-use count_bytes::count_bytes_fast;
+use count_fast::{count_bytes_and_lines_fast, count_bytes_fast};
 use countable::WordCountable;
+use unicode_width::UnicodeWidthChar;
+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;
+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(String),
-}
-
-type WcResult<T> = Result<T, WcError>;
-
 struct Settings {
     show_bytes: bool,
     show_chars: bool,
@@ -114,7 +106,7 @@ enum StdinKind {
 /// Supported inputs.
 enum Input {
     /// A regular file.
-    Path(String),
+    Path(PathBuf),
 
     /// Standard input.
     Stdin(StdinKind),
@@ -122,13 +114,20 @@ enum Input {
 
 impl Input {
     /// Converts input to title that appears in stats.
-    fn to_title(&self) -> Option<&str> {
+    fn to_title(&self) -> Option<&Path> {
         match self {
             Input::Path(path) => Some(path),
-            Input::Stdin(StdinKind::Explicit) => Some("-"),
+            Input::Stdin(StdinKind::Explicit) => Some("-".as_ref()),
             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 {
@@ -137,13 +136,13 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     let matches = uu_app().usage(&usage[..]).get_matches_from(args);
 
     let mut inputs: Vec<Input> = matches
-        .values_of(ARG_FILES)
+        .values_of_os(ARG_FILES)
         .map(|v| {
             v.map(|i| {
                 if i == "-" {
                     Input::Stdin(StdinKind::Explicit)
                 } else {
-                    Input::Path(ToString::to_string(i))
+                    Input::Path(i.into())
                 }
             })
             .collect()
@@ -203,55 +202,125 @@ pub fn uu_app() -> App<'static, 'static> {
 fn word_count_from_reader<T: WordCountable>(
     mut reader: T,
     settings: &Settings,
-    path: &str,
+) -> (WordCount, Option<io::Error>) {
-) -> WcResult<WordCount> {
     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 {
+        let (bytes, error) = count_bytes_fast(&mut reader);
-            bytes: 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
     let decode_chars = settings.show_chars || settings.show_words || settings.show_max_line_length;
 
-    // Sum the WordCount for each line. Show a warning for each line
+    if !decode_chars {
-    // that results in an IO error when trying to read it.
+        return count_bytes_and_lines_fast(&mut reader);
-    let total = reader
-        .lines()
-        .filter_map(|res| match res {
-            Ok(line) => Some(line),
-            Err(e) => {
-                show_warning!("Error while reading {}: {}", path, e);
-                None
     }
-        })
+
-        .map(|line| WordCount::from_line(&line, decode_chars))
+    let mut total = WordCount::default();
-        .sum();
+    let mut reader = BufReadDecoder::new(reader.buffered());
-    Ok(total)
+    let mut in_word = false;
+    let mut current_len = 0;
+
+    while let Some(chunk) = reader.next_strict() {
+        match chunk {
+            Ok(text) => {
+                for ch in text.chars() {
+                    if settings.show_words {
+                        if ch.is_whitespace() {
+                            in_word = false;
+                        } else if ch.is_ascii_control() {
+                            // These count as characters but do not affect the word state
+                        } else if !in_word {
+                            in_word = true;
+                            total.words += 1;
+                        }
+                    }
+                    if settings.show_max_line_length {
+                        match ch {
+                            '\n' => {
+                                total.max_line_length = max(current_len, total.max_line_length);
+                                current_len = 0;
+                            }
+                            // '\x0c' = '\f'
+                            '\r' | '\x0c' => {
+                                total.max_line_length = max(current_len, total.max_line_length);
+                                current_len = 0;
+                            }
+                            '\t' => {
+                                current_len -= current_len % 8;
+                                current_len += 8;
+                            }
+                            _ => {
+                                current_len += ch.width().unwrap_or(0);
+                            }
+                        }
+                    }
+                    if settings.show_lines && ch == '\n' {
+                        total.lines += 1;
+                    }
+                    if settings.show_chars {
+                        total.chars += 1;
+                    }
+                }
+                total.bytes += text.len();
+            }
+            Err(BufReadDecoderError::InvalidByteSequence(bytes)) => {
+                // GNU wc treats invalid data as neither word nor char nor whitespace,
+                // so no other counters are affected
+                total.bytes += bytes.len();
+            }
+            Err(BufReadDecoderError::Io(e)) => {
+                return (total, Some(e));
+            }
+        }
+    }
+
+    total.max_line_length = max(current_len, total.max_line_length);
+
+    (total, None)
 }
 
-fn word_count_from_input(input: &Input, settings: &Settings) -> WcResult<WordCount> {
+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<io::Error>).
+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, "-")
+            match word_count_from_reader(stdin_lock, settings) {
-        }
+                (total, Some(error)) => CountResult::Interrupted(total, error),
-        Input::Path(path) => {
+                (total, None) => CountResult::Success(total),
-            let path_obj = Path::new(path);
-            if path_obj.is_dir() {
-                Err(WcError::IsDirectory(path.to_owned()))
-            } else {
-                let file = File::open(path)?;
-                word_count_from_reader(file, settings, path)
             }
         }
+        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),
+            },
+        },
     }
 }
 
@@ -264,18 +333,8 @@ fn word_count_from_input(input: &Input, settings: &Settings) -> WcResult<WordCou
 /// ```rust,ignore
 /// show_error(Input::Path("/tmp"), WcError::IsDirectory("/tmp"))
 /// ```
-fn show_error(input: &Input, err: WcError) {
+fn show_error(input: &Input, err: io::Error) {
-    match (input, err) {
+    show_error!("{}: {}", input.path_display(), err);
-        (_, WcError::IsDirectory(path)) => {
-            show_error_custom_description!(path, "Is a directory");
-        }
-        (Input::Path(path), WcError::Io(e)) if e.kind() == ErrorKind::NotFound => {
-            show_error_custom_description!(path, "No such file or directory");
-        }
-        (_, e) => {
-            show_error!("{}", e);
-        }
-    };
 }
 
 /// Compute the number of digits needed to represent any count for this input.
@@ -297,9 +356,9 @@ 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<Option<usize>> {
+fn digit_width(input: &Input) -> io::Result<usize> {
     match input {
-        Input::Stdin(_) => Ok(Some(MINIMUM_WIDTH)),
+        Input::Stdin(_) => Ok(MINIMUM_WIDTH),
         Input::Path(filename) => {
             let path = Path::new(filename);
             let metadata = fs::metadata(path)?;
@@ -310,9 +369,9 @@ fn digit_width(input: &Input) -> WcResult<Option<usize>> {
                 // instead). See GitHub issue #2201.
                 let num_bytes = metadata.len();
                 let num_digits = num_bytes.to_string().len();
-                Ok(Some(num_digits))
+                Ok(num_digits)
             } else {
-                Ok(None)
+                Ok(MINIMUM_WIDTH)
             }
         }
     }
@@ -350,7 +409,7 @@ fn digit_width(input: &Input) -> WcResult<Option<usize>> {
 fn max_width(inputs: &[Input]) -> usize {
     let mut result = 1;
     for input in inputs {
-        if let Ok(Some(n)) = digit_width(input) {
+        if let Ok(n) = digit_width(input) {
             result = result.max(n);
         }
     }
@@ -363,7 +422,7 @@ fn wc(inputs: Vec<Input>, 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();
@@ -371,35 +430,43 @@ fn wc(inputs: Vec<Input>, 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| {
+        let word_count = match word_count_from_input(input, settings) {
-            show_error(input, err);
+            CountResult::Success(word_count) => word_count,
-            error_count += 1;
+            CountResult::Interrupted(word_count, error) => {
-            WordCount::default()
+                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) {
             show_warning!(
                 "failed to print result for {}: {}",
-                result.title.unwrap_or("<stdin>"),
+                result.title.unwrap_or_else(|| "<stdin>".as_ref()).display(),
                 err
             );
-            error_count += 1;
+            failure = true;
         }
     }
 
     if num_inputs > 1 {
-        let total_result = total_word_count.with_title(Some("total"));
+        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 {
+    if failure {
-        Ok(())
+        Err(1)
     } else {
-        Err(error_count)
+        Ok(())
     }
 }
 
@@ -407,7 +474,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();
 
@@ -433,13 +500,6 @@ fn print_stats(
         write!(stdout_lock, "{:1$}", result.count.words, min_width)?;
         is_first = false;
     }
-    if settings.show_bytes {
-        if !is_first {
-            write!(stdout_lock, " ")?;
-        }
-        write!(stdout_lock, "{:1$}", result.count.bytes, min_width)?;
-        is_first = false;
-    }
     if settings.show_chars {
         if !is_first {
             write!(stdout_lock, " ")?;
@@ -447,6 +507,13 @@ fn print_stats(
         write!(stdout_lock, "{:1$}", result.count.chars, min_width)?;
         is_first = false;
     }
+    if settings.show_bytes {
+        if !is_first {
+            write!(stdout_lock, " ")?;
+        }
+        write!(stdout_lock, "{:1$}", result.count.bytes, min_width)?;
+        is_first = false;
+    }
     if settings.show_max_line_length {
         if !is_first {
             write!(stdout_lock, " ")?;
@@ -459,7 +526,7 @@ fn print_stats(
     }
 
     if let Some(title) = result.title {
-        writeln!(stdout_lock, " {}", title)?;
+        writeln!(stdout_lock, " {}", title.display())?;
     } else {
         writeln!(stdout_lock)?;
     }

src/uu/wc/src/word_count.rs

@@ -1,19 +1,6 @@
 use std::cmp::max;
-use std::iter::Sum;
 use std::ops::{Add, AddAssign};
-use std::str::from_utf8;
+use std::path::Path;
-
-const CR: u8 = b'\r';
-const LF: u8 = b'\n';
-const SPACE: u8 = b' ';
-const TAB: u8 = b'\t';
-const SYN: u8 = 0x16_u8;
-const FF: u8 = 0x0C_u8;
-
-#[inline(always)]
-fn is_word_separator(byte: u8) -> bool {
-    byte == SPACE || byte == TAB || byte == CR || byte == SYN || byte == FF
-}
 
 #[derive(Debug, Default, Copy, Clone)]
 pub struct WordCount {
@@ -44,80 +31,10 @@ impl AddAssign for WordCount {
     }
 }
 
-impl Sum for WordCount {
-    fn sum<I>(iter: I) -> WordCount
-    where
-        I: Iterator<Item = WordCount>,
-    {
-        iter.fold(WordCount::default(), |acc, x| acc + x)
-    }
-}
-
 impl WordCount {
-    /// Count the characters and whitespace-separated words in the given bytes.
+    pub fn with_title(self, title: Option<&Path>) -> TitledWordCount {
-    ///
-    /// `line` is a slice of bytes that will be decoded as ASCII characters.
-    fn ascii_word_and_char_count(line: &[u8]) -> (usize, usize) {
-        let word_count = line.split(|&x| is_word_separator(x)).count();
-        let char_count = line.iter().filter(|c| c.is_ascii()).count();
-        (word_count, char_count)
-    }
-
-    /// Create a [`WordCount`] from a sequence of bytes representing a line.
-    ///
-    /// If the last byte of `line` encodes a newline character (`\n`),
-    /// then the [`lines`] field will be set to 1. Otherwise, it will
-    /// be set to 0. The [`bytes`] field is simply the length of
-    /// `line`.
-    ///
-    /// If `decode_chars` is `false`, the [`chars`] and [`words`]
-    /// fields will be set to 0. If it is `true`, this function will
-    /// attempt to decode the bytes first as UTF-8, and failing that,
-    /// as ASCII.
-    pub fn from_line(line: &[u8], decode_chars: bool) -> WordCount {
-        // GNU 'wc' only counts lines that end in LF as lines
-        let lines = (*line.last().unwrap() == LF) as usize;
-        let bytes = line.len();
-        let (words, chars) = if decode_chars {
-            WordCount::word_and_char_count(line)
-        } else {
-            (0, 0)
-        };
-        // -L is a GNU 'wc' extension so same behavior on LF
-        let max_line_length = if chars > 0 { chars - lines } else { 0 };
-        WordCount {
-            bytes,
-            chars,
-            lines,
-            words,
-            max_line_length,
-        }
-    }
-
-    /// Count the UTF-8 characters and words in the given string slice.
-    ///
-    /// `s` is a string slice that is assumed to be a UTF-8 string.
-    fn utf8_word_and_char_count(s: &str) -> (usize, usize) {
-        let word_count = s.split_whitespace().count();
-        let char_count = s.chars().count();
-        (word_count, char_count)
-    }
-
-    pub fn with_title(self, title: Option<&str>) -> TitledWordCount {
         TitledWordCount { title, count: self }
     }
-
-    /// Count the characters and words in the given slice of bytes.
-    ///
-    /// `line` is a slice of bytes that will be decoded as UTF-8
-    /// characters, or if that fails, as ASCII characters.
-    fn word_and_char_count(line: &[u8]) -> (usize, usize) {
-        // try and convert the bytes to UTF-8 first
-        match from_utf8(line) {
-            Ok(s) => WordCount::utf8_word_and_char_count(s),
-            Err(..) => WordCount::ascii_word_and_char_count(line),
-        }
-    }
 }
 
 /// This struct supplements the actual word count with an optional title that is
@@ -126,6 +43,6 @@ impl WordCount {
 /// it would result in unnecessary copying of `String`.
 #[derive(Debug, Default, Clone)]
 pub struct TitledWordCount<'a> {
-    pub title: Option<&'a str>,
+    pub title: Option<&'a Path>,
     pub count: WordCount,
 }

src/uu/who/src/who.rs

@@ -492,7 +492,7 @@ impl Who {
         };
 
         let s = if self.do_lookup {
-            safe_unwrap!(ut.canon_host())
+            crash_if_err!(1, ut.canon_host())
         } else {
             ut.host()
         };

src/uucore/Cargo.toml

@@ -16,6 +16,7 @@ edition = "2018"
 path="src/lib/lib.rs"
 
 [dependencies]
+clap = "2.33.3"
 dns-lookup = { version="1.0.5", optional=true }
 dunce = "1.0.0"
 getopts = "<= 0.2.21"

src/uucore/src/lib/features/fs.rs

@@ -6,6 +6,8 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
+//! Set of functions to manage files and symlinks
+
 #[cfg(unix)]
 use libc::{
     mode_t, S_IFBLK, S_IFCHR, S_IFDIR, S_IFIFO, S_IFLNK, S_IFMT, S_IFREG, S_IFSOCK, S_IRGRP,
@@ -15,6 +17,7 @@ use libc::{
 use std::borrow::Cow;
 use std::env;
 use std::fs;
+use std::io::Error as IOError;
 use std::io::Result as IOResult;
 use std::io::{Error, ErrorKind};
 #[cfg(any(unix, target_os = "redox"))]
@@ -109,27 +112,43 @@ pub fn normalize_path(path: &Path) -> PathBuf {
     ret
 }
 
-fn resolve<P: AsRef<Path>>(original: P) -> IOResult<PathBuf> {
+fn resolve<P: AsRef<Path>>(original: P) -> Result<PathBuf, (PathBuf, IOError)> {
     const MAX_LINKS_FOLLOWED: u32 = 255;
     let mut followed = 0;
     let mut result = original.as_ref().to_path_buf();
+
+    let mut first_resolution = None;
     loop {
         if followed == MAX_LINKS_FOLLOWED {
-            return Err(Error::new(
+            return Err((
-                ErrorKind::InvalidInput,
+                // When we hit MAX_LINKS_FOLLOWED we should return the first resolution (that's what GNU does - for whatever reason)
-                "maximum links followed",
+                first_resolution.unwrap(),
+                Error::new(ErrorKind::InvalidInput, "maximum links followed"),
             ));
         }
 
-        if !fs::symlink_metadata(&result)?.file_type().is_symlink() {
+        match fs::symlink_metadata(&result) {
+            Ok(meta) => {
+                if !meta.file_type().is_symlink() {
                     break;
                 }
+            }
+            Err(e) => return Err((result, e)),
+        }
 
         followed += 1;
-        let path = fs::read_link(&result)?;
+        match fs::read_link(&result) {
+            Ok(path) => {
                 result.pop();
                 result.push(path);
             }
+            Err(e) => return Err((result, e)),
+        }
+
+        if first_resolution.is_none() {
+            first_resolution = Some(result.clone());
+        }
+    }
     Ok(result)
 }
 
@@ -214,11 +233,10 @@ pub fn canonicalize<P: AsRef<Path>>(
             }
 
             match resolve(&result) {
-                Err(_) if miss_mode == MissingHandling::Missing => continue,
+                Err((path, _)) if miss_mode == MissingHandling::Missing => result = path,
-                Err(e) => return Err(e),
+                Err((_, e)) => return Err(e),
                 Ok(path) => {
-                    result.pop();
+                    result = path;
-                    result.push(path);
                 }
             }
         }
@@ -230,14 +248,12 @@ pub fn canonicalize<P: AsRef<Path>>(
         }
 
         match resolve(&result) {
-            Err(e) if miss_mode == MissingHandling::Existing => {
+            Err((_, e)) if miss_mode == MissingHandling::Existing => {
                 return Err(e);
             }
-            Ok(path) => {
+            Ok(path) | Err((path, _)) => {
-                result.pop();
+                result = path;
-                result.push(path);
             }
-            Err(_) => (),
         }
         if res_mode == ResolveMode::Physical {
             result = normalize_path(&result);

src/uucore/src/lib/features/fsext.rs

@@ -7,6 +7,8 @@
 // For the full copyright and license information, please view the LICENSE file
 // that was distributed with this source code.
 
+//! Set of functions to manage file systems
+
 // spell-checker:ignore (arch) bitrig ; (fs) cifs smbfs
 
 extern crate time;

src/uucore/src/lib/features/mode.rs

@@ -5,6 +5,8 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
+//! Set of functions to parse modes
+
 // spell-checker:ignore (vars) fperm srwx
 
 use libc::{mode_t, umask, S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR};

src/uucore/src/lib/features/perms.rs

@@ -3,6 +3,8 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
+//! Common functions to manage permissions
+
 use crate::error::UResult;
 pub use crate::features::entries;
 use crate::fs::resolve_relative_path;

src/uucore/src/lib/features/process.rs

@@ -9,6 +9,8 @@
 // spell-checker:ignore (vars) cvar exitstatus
 // spell-checker:ignore (sys/unix) WIFSIGNALED
 
+//! Set of functions to manage IDs
+
 use libc::{gid_t, pid_t, uid_t};
 use std::fmt;
 use std::io;

src/uucore/src/lib/macros.rs

@@ -99,24 +99,6 @@ macro_rules! crash_if_err(
 
 //====
 
-/// Unwraps the Result. Instead of panicking, it shows the error and then
-/// returns from the function with the provided exit code.
-/// Assumes the current function returns an i32 value.
-#[macro_export]
-macro_rules! return_if_err(
-    ($exit_code:expr, $exp:expr) => (
-        match $exp {
-            Ok(m) => m,
-            Err(f) => {
-                $crate::show_error!("{}", f);
-                return $exit_code;
-            }
-        }
-    )
-);
-
-//====
-
 #[macro_export]
 macro_rules! safe_write(
     ($fd:expr, $($args:tt)+) => (
@@ -137,18 +119,6 @@ macro_rules! safe_writeln(
     )
 );
 
-/// Unwraps the Result. Instead of panicking, it exists the program with exit
-/// code 1.
-#[macro_export]
-macro_rules! safe_unwrap(
-    ($exp:expr) => (
-        match $exp {
-            Ok(m) => m,
-            Err(f) => $crate::crash!(1, "{}", f.to_string())
-        }
-    )
-);
-
 //-- message templates
 
 //-- message templates : (join utility sub-macros)

src/uucore/src/lib/mods/backup_control.rs

@@ -1,5 +1,89 @@
+//! Implement GNU-style backup functionality.
+//!
+//! This module implements the backup functionality as described in the [GNU
+//! manual][1]. It provides
+//!
+//! - pre-defined [`clap`-Arguments][2] for inclusion in utilities that
+//!   implement backups
+//! - determination of the [backup mode][3]
+//! - determination of the [backup suffix][4]
+//! - [backup target path construction][5]
+//! - [Error types][6] for backup-related errors
+//! - GNU-compliant [help texts][7] for backup-related errors
+//!
+//! Backup-functionality is implemented by the following utilities:
+//!
+//! - `cp`
+//! - `install`
+//! - `ln`
+//! - `mv`
+//!
+//!
+//! [1]: https://www.gnu.org/software/coreutils/manual/html_node/Backup-options.html
+//! [2]: arguments
+//! [3]: `determine_backup_mode()`
+//! [4]: `determine_backup_suffix()`
+//! [5]: `get_backup_path()`
+//! [6]: `BackupError`
+//! [7]: `BACKUP_CONTROL_LONG_HELP`
+//!
+//!
+//! # Usage example
+//!
+//! ```
+//! #[macro_use]
+//! extern crate uucore;
+//!
+//! use clap::{App, Arg, ArgMatches};
+//! use std::path::{Path, PathBuf};
+//! use uucore::backup_control::{self, BackupMode};
+//! use uucore::error::{UError, UResult};
+//!
+//! fn main() {
+//!     let usage = String::from("app [OPTION]... ARG");
+//!     let long_usage = String::from("And here's a detailed explanation");
+//!
+//!     let matches = App::new("app")
+//!         .arg(backup_control::arguments::backup())
+//!         .arg(backup_control::arguments::backup_no_args())
+//!         .arg(backup_control::arguments::suffix())
+//!         .usage(&usage[..])
+//!         .after_help(&*format!(
+//!             "{}\n{}",
+//!             long_usage,
+//!             backup_control::BACKUP_CONTROL_LONG_HELP
+//!         ))
+//!         .get_matches_from(vec![
+//!             "app", "--backup=t", "--suffix=bak~"
+//!         ]);
+//!    
+//!     let backup_mode = match backup_control::determine_backup_mode(&matches) {
+//!         Err(e) => {
+//!             show!(e);
+//!             return;
+//!         },
+//!         Ok(mode) => mode,
+//!     };
+//!     let backup_suffix = backup_control::determine_backup_suffix(&matches);
+//!     let target_path = Path::new("/tmp/example");
+//!
+//!     let backup_path = backup_control::get_backup_path(
+//!         backup_mode, target_path, &backup_suffix
+//!     );
+//!
+//!     // Perform your backups here.
+//!
+//! }
+//! ```
+
+// spell-checker:ignore backupopt
+
+use crate::error::{UError, UResult};
+use clap::ArgMatches;
 use std::{
     env,
+    error::Error,
+    fmt::{Debug, Display},
     path::{Path, PathBuf},
 };
 
@@ -17,15 +101,144 @@ the VERSION_CONTROL environment variable.  Here are the values:
   existing, nil   numbered if numbered backups exist, simple otherwise
   simple, never   always make simple backups";
 
+static VALID_ARGS_HELP: &str = "Valid arguments are:
+  - 'none', 'off'
+  - 'simple', 'never'
+  - 'existing', 'nil'
+  - 'numbered', 't'";
+
+/// Available backup modes.
+///
+/// The mapping of the backup modes to the CLI arguments is annotated on the
+/// enum variants.
 #[derive(Debug, Clone, Copy, Eq, PartialEq)]
 pub enum BackupMode {
+    /// Argument 'none', 'off'
     NoBackup,
+    /// Argument 'simple', 'never'
     SimpleBackup,
+    /// Argument 'numbered', 't'
     NumberedBackup,
+    /// Argument 'existing', 'nil'
     ExistingBackup,
 }
 
-pub fn determine_backup_suffix(supplied_suffix: Option<&str>) -> String {
+/// Backup error types.
+///
+/// Errors are currently raised by [`determine_backup_mode`] only. All errors
+/// are implemented as [`UCustomError`] for uniform handling across utilities.
+#[derive(Debug, Eq, PartialEq)]
+pub enum BackupError {
+    /// An invalid argument (e.g. 'foo') was given as backup type. First
+    /// parameter is the argument, second is the arguments origin (CLI or
+    /// ENV-var)
+    InvalidArgument(String, String),
+    /// An ambiguous argument (e.g. 'n') was given as backup type. First
+    /// parameter is the argument, second is the arguments origin (CLI or
+    /// ENV-var)
+    AmbiguousArgument(String, String),
+    /// Currently unused
+    BackupImpossible(),
+    // BackupFailed(PathBuf, PathBuf, std::io::Error),
+}
+
+impl UError for BackupError {
+    fn code(&self) -> i32 {
+        match self {
+            BackupError::BackupImpossible() => 2,
+            _ => 1,
+        }
+    }
+
+    fn usage(&self) -> bool {
+        // Suggested by clippy.
+        matches!(
+            self,
+            BackupError::InvalidArgument(_, _) | BackupError::AmbiguousArgument(_, _)
+        )
+    }
+}
+
+impl Error for BackupError {}
+
+impl Display for BackupError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        use BackupError as BE;
+        match self {
+            BE::InvalidArgument(arg, origin) => write!(
+                f,
+                "invalid argument '{}' for '{}'\n{}",
+                arg, origin, VALID_ARGS_HELP
+            ),
+            BE::AmbiguousArgument(arg, origin) => write!(
+                f,
+                "ambiguous argument '{}' for '{}'\n{}",
+                arg, origin, VALID_ARGS_HELP
+            ),
+            BE::BackupImpossible() => write!(f, "cannot create backup"),
+            // Placeholder for later
+            // BE::BackupFailed(from, to, e) => Display::fmt(
+            //     &uio_error!(e, "failed to backup '{}' to '{}'", from.display(), to.display()),
+            //     f
+            // ),
+        }
+    }
+}
+
+/// Arguments for backup-related functionality.
+///
+/// Rather than implementing the `clap`-Arguments for every utility, it is
+/// recommended to include the `clap` arguments via the functions provided here.
+/// This way the backup-specific arguments are handled uniformly across
+/// utilities and can be maintained in one central place.
+pub mod arguments {
+    extern crate clap;
+
+    pub static OPT_BACKUP: &str = "backupopt_backup";
+    pub static OPT_BACKUP_NO_ARG: &str = "backupopt_b";
+    pub static OPT_SUFFIX: &str = "backupopt_suffix";
+
+    /// '--backup' argument
+    pub fn backup() -> clap::Arg<'static, 'static> {
+        clap::Arg::with_name(OPT_BACKUP)
+            .long("backup")
+            .help("make a backup of each existing destination file")
+            .takes_value(true)
+            .require_equals(true)
+            .min_values(0)
+            .value_name("CONTROL")
+    }
+
+    /// '-b' argument
+    pub fn backup_no_args() -> clap::Arg<'static, 'static> {
+        clap::Arg::with_name(OPT_BACKUP_NO_ARG)
+            .short("b")
+            .help("like --backup but does not accept an argument")
+    }
+
+    /// '-S, --suffix' argument
+    pub fn suffix() -> clap::Arg<'static, 'static> {
+        clap::Arg::with_name(OPT_SUFFIX)
+            .short("S")
+            .long("suffix")
+            .help("override the usual backup suffix")
+            .takes_value(true)
+            .value_name("SUFFIX")
+    }
+}
+
+/// Obtain the suffix to use for a backup.
+///
+/// In order of precedence, this function obtains the backup suffix
+///
+/// 1. From the '-S' or '--suffix' CLI argument, if present
+/// 2. From the "SIMPLE_BACKUP_SUFFIX" environment variable, if present
+/// 3. By using the default '~' if none of the others apply
+///
+/// This function directly takes [`clap::ArgMatches`] as argument and looks for
+/// the '-S' and '--suffix' arguments itself.
+pub fn determine_backup_suffix(matches: &ArgMatches) -> String {
+    let supplied_suffix = matches.value_of(arguments::OPT_SUFFIX);
     if let Some(suffix) = supplied_suffix {
         String::from(suffix)
     } else {
@@ -38,7 +251,13 @@ pub fn determine_backup_suffix(supplied_suffix: Option<&str>) -> String {
 /// Parses the backup options according to the [GNU manual][1], and converts
 /// them to an instance of `BackupMode` for further processing.
 ///
-/// For an explanation of what the arguments mean, refer to the examples below.
+/// Takes [`clap::ArgMatches`] as argument which **must** contain the options
+/// from [`arguments::backup()`] and [`arguments::backup_no_args()`]. Otherwise
+/// the `NoBackup` mode is returned unconditionally.
+///
+/// It is recommended for anyone who would like to implement the
+/// backup-functionality to use the arguments prepared in the `arguments`
+/// submodule (see examples)
 ///
 /// [1]: https://www.gnu.org/software/coreutils/manual/html_node/Backup-options.html
 ///
@@ -47,9 +266,11 @@ pub fn determine_backup_suffix(supplied_suffix: Option<&str>) -> String {
 ///
 /// If an argument supplied directly to the long `backup` option, or read in
 /// through the `VERSION CONTROL` env var is ambiguous (i.e. may resolve to
-/// multiple backup modes) or invalid, an error is returned. The error contains
+/// multiple backup modes) or invalid, an [`InvalidArgument`][10] or
-/// the formatted error string which may then be passed to the
+/// [`AmbiguousArgument`][11] error is returned, respectively.
-/// [`show_usage_error`] macro.
+///
+/// [10]: BackupError::InvalidArgument
+/// [11]: BackupError::AmbiguousArgument
 ///
 ///
 /// # Examples
@@ -61,34 +282,18 @@ pub fn determine_backup_suffix(supplied_suffix: Option<&str>) -> String {
 /// #[macro_use]
 /// extern crate uucore;
 /// use uucore::backup_control::{self, BackupMode};
-/// use clap::{App, Arg};
+/// use clap::{App, Arg, ArgMatches};
 ///
 /// fn main() {
-///     let OPT_BACKUP: &str = "backup";
-///     let OPT_BACKUP_NO_ARG: &str = "b";
 ///     let matches = App::new("app")
-///         .arg(Arg::with_name(OPT_BACKUP_NO_ARG)
+///         .arg(backup_control::arguments::backup())
-///             .short(OPT_BACKUP_NO_ARG))
+///         .arg(backup_control::arguments::backup_no_args())
-///         .arg(Arg::with_name(OPT_BACKUP)
-///             .long(OPT_BACKUP)
-///             .takes_value(true)
-///             .require_equals(true)
-///             .min_values(0))
 ///         .get_matches_from(vec![
 ///             "app", "-b", "--backup=t"
 ///         ]);
 ///
-///     let backup_mode = backup_control::determine_backup_mode(
+///     let backup_mode = backup_control::determine_backup_mode(&matches).unwrap();
-///         matches.is_present(OPT_BACKUP_NO_ARG), matches.is_present(OPT_BACKUP),
+///     assert_eq!(backup_mode, BackupMode::NumberedBackup)
-///         matches.value_of(OPT_BACKUP)
-///     );
-///     let backup_mode = match backup_mode {
-///         Err(err) => {
-///             show_usage_error!("{}", err);
-///             return;
-///         },
-///         Ok(mode) => mode,
-///     };
 /// }
 /// ```
 ///
@@ -99,57 +304,43 @@ pub fn determine_backup_suffix(supplied_suffix: Option<&str>) -> String {
 /// ```
 /// #[macro_use]
 /// extern crate uucore;
-/// use uucore::backup_control::{self, BackupMode};
+/// use uucore::backup_control::{self, BackupMode, BackupError};
-/// use clap::{crate_version, App, Arg, ArgMatches};
+/// use clap::{App, Arg, ArgMatches};
 ///
 /// fn main() {
-///     let OPT_BACKUP: &str = "backup";
-///     let OPT_BACKUP_NO_ARG: &str = "b";
 ///     let matches = App::new("app")
-///         .arg(Arg::with_name(OPT_BACKUP_NO_ARG)
+///         .arg(backup_control::arguments::backup())
-///             .short(OPT_BACKUP_NO_ARG))
+///         .arg(backup_control::arguments::backup_no_args())
-///         .arg(Arg::with_name(OPT_BACKUP)
-///             .long(OPT_BACKUP)
-///             .takes_value(true)
-///             .require_equals(true)
-///             .min_values(0))
 ///         .get_matches_from(vec![
 ///             "app", "-b", "--backup=n"
 ///         ]);
 ///    
-///     let backup_mode = backup_control::determine_backup_mode(
+///     let backup_mode = backup_control::determine_backup_mode(&matches);
-///         matches.is_present(OPT_BACKUP_NO_ARG), matches.is_present(OPT_BACKUP),
+///
-///         matches.value_of(OPT_BACKUP)
+///     assert!(backup_mode.is_err());
-///     );
+///     let err = backup_mode.unwrap_err();
-///     let backup_mode = match backup_mode {
+///     // assert_eq!(err, BackupError::AmbiguousArgument);
-///         Err(err) => {
+///     // Use uucore functionality to show the error to the user
-///             show_usage_error!("{}", err);
+///     show!(err);
-///             return;
-///         },
-///         Ok(mode) => mode,
-///     };
 /// }
 /// ```
-pub fn determine_backup_mode(
+pub fn determine_backup_mode(matches: &ArgMatches) -> UResult<BackupMode> {
-    short_opt_present: bool,
+    if matches.is_present(arguments::OPT_BACKUP) {
-    long_opt_present: bool,
-    long_opt_value: Option<&str>,
-) -> Result<BackupMode, String> {
-    if long_opt_present {
         // Use method to determine the type of backups to make. When this option
         // is used but method is not specified, then the value of the
         // VERSION_CONTROL environment variable is used. And if VERSION_CONTROL
-        // is not set, the default backup type is ‘existing’.
+        // is not set, the default backup type is 'existing'.
-        if let Some(method) = long_opt_value {
+        if let Some(method) = matches.value_of(arguments::OPT_BACKUP) {
             // Second argument is for the error string that is returned.
             match_method(method, "backup type")
         } else if let Ok(method) = env::var("VERSION_CONTROL") {
             // Second argument is for the error string that is returned.
             match_method(&method, "$VERSION_CONTROL")
         } else {
+            // Default if no argument is provided to '--backup'
             Ok(BackupMode::ExistingBackup)
         }
-    } else if short_opt_present {
+    } else if matches.is_present(arguments::OPT_BACKUP_NO_ARG) {
         // the short form of this option, -b does not accept any argument.
         // Using -b is equivalent to using --backup=existing.
         Ok(BackupMode::ExistingBackup)
@@ -172,10 +363,13 @@ pub fn determine_backup_mode(
 ///
 /// # Errors
 ///
-/// If `method` is ambiguous (i.e. may resolve to multiple backup modes) or
+/// If `method` is invalid or ambiguous (i.e. may resolve to multiple backup
-/// invalid, an error is returned. The error contains the formatted error string
+/// modes), an [`InvalidArgument`][10] or [`AmbiguousArgument`][11] error is
-/// which may then be passed to the [`show_usage_error`] macro.
+/// returned, respectively.
-fn match_method(method: &str, origin: &str) -> Result<BackupMode, String> {
+///
+/// [10]: BackupError::InvalidArgument
+/// [11]: BackupError::AmbiguousArgument
+fn match_method(method: &str, origin: &str) -> UResult<BackupMode> {
     let matches: Vec<&&str> = BACKUP_CONTROL_VALUES
         .iter()
         .filter(|val| val.starts_with(method))
@@ -189,21 +383,10 @@ fn match_method(method: &str, origin: &str) -> Result<BackupMode, String> {
             _ => unreachable!(), // cannot happen as we must have exactly one match
                                  // from the list above.
         }
+    } else if matches.is_empty() {
+        Err(BackupError::InvalidArgument(method.to_string(), origin.to_string()).into())
     } else {
-        let error_type = if matches.is_empty() {
+        Err(BackupError::AmbiguousArgument(method.to_string(), origin.to_string()).into())
-            "invalid"
-        } else {
-            "ambiguous"
-        };
-        Err(format!(
-            "{0} argument ‘{1}’ for ‘{2}’
-Valid arguments are:
-  - ‘none’, ‘off’
-  - ‘simple’, ‘never’
-  - ‘existing’, ‘nil’
-  - ‘numbered’, ‘t’",
-            error_type, method, origin
-        ))
     }
 }
 
@@ -255,6 +438,7 @@ mod tests {
     use super::*;
     use std::env;
     // Required to instantiate mutex in shared context
+    use clap::App;
     use lazy_static::lazy_static;
     use std::sync::Mutex;
 
@@ -271,16 +455,20 @@ mod tests {
     // Environment variable for "VERSION_CONTROL"
     static ENV_VERSION_CONTROL: &str = "VERSION_CONTROL";
 
+    fn make_app() -> clap::App<'static, 'static> {
+        App::new("app")
+            .arg(arguments::backup())
+            .arg(arguments::backup_no_args())
+            .arg(arguments::suffix())
+    }
+
     // Defaults to --backup=existing
     #[test]
     fn test_backup_mode_short_only() {
-        let short_opt_present = true;
-        let long_opt_present = false;
-        let long_opt_value = None;
         let _dummy = TEST_MUTEX.lock().unwrap();
+        let matches = make_app().get_matches_from(vec!["app", "-b"]);
 
-        let result =
+        let result = determine_backup_mode(&matches).unwrap();
-            determine_backup_mode(short_opt_present, long_opt_present, long_opt_value).unwrap();
 
         assert_eq!(result, BackupMode::ExistingBackup);
     }
@@ -288,13 +476,10 @@ mod tests {
     // --backup takes precedence over -b
     #[test]
     fn test_backup_mode_long_preferred_over_short() {
-        let short_opt_present = true;
-        let long_opt_present = true;
-        let long_opt_value = Some("none");
         let _dummy = TEST_MUTEX.lock().unwrap();
+        let matches = make_app().get_matches_from(vec!["app", "-b", "--backup=none"]);
 
-        let result =
+        let result = determine_backup_mode(&matches).unwrap();
-            determine_backup_mode(short_opt_present, long_opt_present, long_opt_value).unwrap();
 
         assert_eq!(result, BackupMode::NoBackup);
     }
@@ -302,13 +487,10 @@ mod tests {
     // --backup can be passed without an argument
     #[test]
     fn test_backup_mode_long_without_args_no_env() {
-        let short_opt_present = false;
-        let long_opt_present = true;
-        let long_opt_value = None;
         let _dummy = TEST_MUTEX.lock().unwrap();
+        let matches = make_app().get_matches_from(vec!["app", "--backup"]);
 
-        let result =
+        let result = determine_backup_mode(&matches).unwrap();
-            determine_backup_mode(short_opt_present, long_opt_present, long_opt_value).unwrap();
 
         assert_eq!(result, BackupMode::ExistingBackup);
     }
@@ -316,13 +498,10 @@ mod tests {
     // --backup can be passed with an argument only
     #[test]
     fn test_backup_mode_long_with_args() {
-        let short_opt_present = false;
-        let long_opt_present = true;
-        let long_opt_value = Some("simple");
         let _dummy = TEST_MUTEX.lock().unwrap();
+        let matches = make_app().get_matches_from(vec!["app", "--backup=simple"]);
 
-        let result =
+        let result = determine_backup_mode(&matches).unwrap();
-            determine_backup_mode(short_opt_present, long_opt_present, long_opt_value).unwrap();
 
         assert_eq!(result, BackupMode::SimpleBackup);
     }
@@ -330,43 +509,36 @@ mod tests {
     // --backup errors on invalid argument
     #[test]
     fn test_backup_mode_long_with_args_invalid() {
-        let short_opt_present = false;
-        let long_opt_present = true;
-        let long_opt_value = Some("foobar");
         let _dummy = TEST_MUTEX.lock().unwrap();
+        let matches = make_app().get_matches_from(vec!["app", "--backup=foobar"]);
 
-        let result = determine_backup_mode(short_opt_present, long_opt_present, long_opt_value);
+        let result = determine_backup_mode(&matches);
 
         assert!(result.is_err());
-        let text = result.unwrap_err();
+        let text = format!("{}", result.unwrap_err());
-        assert!(text.contains("invalid argument ‘foobar’ for ‘backup type’"));
+        assert!(text.contains("invalid argument 'foobar' for 'backup type'"));
     }
 
     // --backup errors on ambiguous argument
     #[test]
     fn test_backup_mode_long_with_args_ambiguous() {
-        let short_opt_present = false;
-        let long_opt_present = true;
-        let long_opt_value = Some("n");
         let _dummy = TEST_MUTEX.lock().unwrap();
+        let matches = make_app().get_matches_from(vec!["app", "--backup=n"]);
 
-        let result = determine_backup_mode(short_opt_present, long_opt_present, long_opt_value);
+        let result = determine_backup_mode(&matches);
 
         assert!(result.is_err());
-        let text = result.unwrap_err();
+        let text = format!("{}", result.unwrap_err());
-        assert!(text.contains("ambiguous argument ‘n’ for ‘backup type’"));
+        assert!(text.contains("ambiguous argument 'n' for 'backup type'"));
     }
 
     // --backup accepts shortened arguments (si for simple)
     #[test]
     fn test_backup_mode_long_with_arg_shortened() {
-        let short_opt_present = false;
-        let long_opt_present = true;
-        let long_opt_value = Some("si");
         let _dummy = TEST_MUTEX.lock().unwrap();
+        let matches = make_app().get_matches_from(vec!["app", "--backup=si"]);
 
-        let result =
+        let result = determine_backup_mode(&matches).unwrap();
-            determine_backup_mode(short_opt_present, long_opt_present, long_opt_value).unwrap();
 
         assert_eq!(result, BackupMode::SimpleBackup);
     }
@@ -374,14 +546,11 @@ mod tests {
     // -b ignores the "VERSION_CONTROL" environment variable
     #[test]
     fn test_backup_mode_short_only_ignore_env() {
-        let short_opt_present = true;
-        let long_opt_present = false;
-        let long_opt_value = None;
         let _dummy = TEST_MUTEX.lock().unwrap();
         env::set_var(ENV_VERSION_CONTROL, "none");
+        let matches = make_app().get_matches_from(vec!["app", "-b"]);
 
-        let result =
+        let result = determine_backup_mode(&matches).unwrap();
-            determine_backup_mode(short_opt_present, long_opt_present, long_opt_value).unwrap();
 
         assert_eq!(result, BackupMode::ExistingBackup);
         env::remove_var(ENV_VERSION_CONTROL);
@@ -390,14 +559,11 @@ mod tests {
     // --backup can be passed without an argument, but reads env var if existent
     #[test]
     fn test_backup_mode_long_without_args_with_env() {
-        let short_opt_present = false;
-        let long_opt_present = true;
-        let long_opt_value = None;
         let _dummy = TEST_MUTEX.lock().unwrap();
         env::set_var(ENV_VERSION_CONTROL, "none");
+        let matches = make_app().get_matches_from(vec!["app", "--backup"]);
 
-        let result =
+        let result = determine_backup_mode(&matches).unwrap();
-            determine_backup_mode(short_opt_present, long_opt_present, long_opt_value).unwrap();
 
         assert_eq!(result, BackupMode::NoBackup);
         env::remove_var(ENV_VERSION_CONTROL);
@@ -406,48 +572,41 @@ mod tests {
     // --backup errors on invalid VERSION_CONTROL env var
     #[test]
     fn test_backup_mode_long_with_env_var_invalid() {
-        let short_opt_present = false;
-        let long_opt_present = true;
-        let long_opt_value = None;
         let _dummy = TEST_MUTEX.lock().unwrap();
         env::set_var(ENV_VERSION_CONTROL, "foobar");
+        let matches = make_app().get_matches_from(vec!["app", "--backup"]);
 
-        let result = determine_backup_mode(short_opt_present, long_opt_present, long_opt_value);
+        let result = determine_backup_mode(&matches);
 
         assert!(result.is_err());
-        let text = result.unwrap_err();
+        let text = format!("{}", result.unwrap_err());
-        assert!(text.contains("invalid argument ‘foobar’ for ‘$VERSION_CONTROL’"));
+        assert!(text.contains("invalid argument 'foobar' for '$VERSION_CONTROL'"));
         env::remove_var(ENV_VERSION_CONTROL);
     }
 
     // --backup errors on ambiguous VERSION_CONTROL env var
     #[test]
     fn test_backup_mode_long_with_env_var_ambiguous() {
-        let short_opt_present = false;
-        let long_opt_present = true;
-        let long_opt_value = None;
         let _dummy = TEST_MUTEX.lock().unwrap();
         env::set_var(ENV_VERSION_CONTROL, "n");
+        let matches = make_app().get_matches_from(vec!["app", "--backup"]);
 
-        let result = determine_backup_mode(short_opt_present, long_opt_present, long_opt_value);
+        let result = determine_backup_mode(&matches);
 
         assert!(result.is_err());
-        let text = result.unwrap_err();
+        let text = format!("{}", result.unwrap_err());
-        assert!(text.contains("ambiguous argument ‘n’ for ‘$VERSION_CONTROL’"));
+        assert!(text.contains("ambiguous argument 'n' for '$VERSION_CONTROL'"));
         env::remove_var(ENV_VERSION_CONTROL);
     }
 
     // --backup accepts shortened env vars (si for simple)
     #[test]
     fn test_backup_mode_long_with_env_var_shortened() {
-        let short_opt_present = false;
-        let long_opt_present = true;
-        let long_opt_value = None;
         let _dummy = TEST_MUTEX.lock().unwrap();
         env::set_var(ENV_VERSION_CONTROL, "si");
+        let matches = make_app().get_matches_from(vec!["app", "--backup"]);
 
-        let result =
+        let result = determine_backup_mode(&matches).unwrap();
-            determine_backup_mode(short_opt_present, long_opt_present, long_opt_value).unwrap();
 
         assert_eq!(result, BackupMode::SimpleBackup);
         env::remove_var(ENV_VERSION_CONTROL);

src/uucore/src/lib/mods/error.rs

@@ -409,6 +409,15 @@ impl Display for UIoError {
     }
 }
 
+/// Strip the trailing " (os error XX)" from io error strings.
+pub fn strip_errno(err: &std::io::Error) -> String {
+    let mut msg = err.to_string();
+    if let Some(pos) = msg.find(" (os error ") {
+        msg.drain(pos..);
+    }
+    msg
+}
+
 /// Enables the conversion from [`std::io::Error`] to [`UError`] and from [`std::io::Result`] to
 /// [`UResult`].
 pub trait FromIo<T> {

tests/by-util/test_realpath.rs

@@ -139,3 +139,23 @@ fn test_realpath_logical_mode() {
         .succeeds()
         .stdout_contains("dir1\n");
 }
+
+#[test]
+fn test_realpath_dangling() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.symlink_file("nonexistent-file", "link");
+    ucmd.arg("link")
+        .succeeds()
+        .stdout_only(at.plus_as_string("nonexistent-file\n"));
+}
+
+#[test]
+fn test_realpath_loop() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.symlink_file("2", "1");
+    at.symlink_file("3", "2");
+    at.symlink_file("1", "3");
+    ucmd.arg("1")
+        .succeeds()
+        .stdout_only(at.plus_as_string("2\n"));
+}

tests/by-util/test_rmdir.rs

@@ -1,126 +1,238 @@
 use crate::common::util::*;
 
+const DIR: &str = "dir";
+const DIR_FILE: &str = "dir/file";
+const NESTED_DIR: &str = "dir/ect/ory";
+const NESTED_DIR_FILE: &str = "dir/ect/ory/file";
+
+#[cfg(windows)]
+const NOT_FOUND: &str = "The system cannot find the file specified.";
+#[cfg(not(windows))]
+const NOT_FOUND: &str = "No such file or directory";
+
+#[cfg(windows)]
+const NOT_EMPTY: &str = "The directory is not empty.";
+#[cfg(not(windows))]
+const NOT_EMPTY: &str = "Directory not empty";
+
+#[cfg(windows)]
+const NOT_A_DIRECTORY: &str = "The directory name is invalid.";
+#[cfg(not(windows))]
+const NOT_A_DIRECTORY: &str = "Not a directory";
+
 #[test]
 fn test_rmdir_empty_directory_no_parents() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let dir = "test_rmdir_empty_no_parents";
 
-    at.mkdir(dir);
+    at.mkdir(DIR);
-    assert!(at.dir_exists(dir));
 
-    ucmd.arg(dir).succeeds().no_stderr();
+    ucmd.arg(DIR).succeeds().no_stderr();
 
-    assert!(!at.dir_exists(dir));
+    assert!(!at.dir_exists(DIR));
 }
 
 #[test]
 fn test_rmdir_empty_directory_with_parents() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let dir = "test_rmdir_empty/with/parents";
 
-    at.mkdir_all(dir);
+    at.mkdir_all(NESTED_DIR);
-    assert!(at.dir_exists(dir));
 
-    ucmd.arg("-p").arg(dir).succeeds().no_stderr();
+    ucmd.arg("-p").arg(NESTED_DIR).succeeds().no_stderr();
 
-    assert!(!at.dir_exists(dir));
+    assert!(!at.dir_exists(NESTED_DIR));
+    assert!(!at.dir_exists(DIR));
 }
 
 #[test]
 fn test_rmdir_nonempty_directory_no_parents() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let dir = "test_rmdir_nonempty_no_parents";
-    let file = "test_rmdir_nonempty_no_parents/foo";
 
-    at.mkdir(dir);
+    at.mkdir(DIR);
-    assert!(at.dir_exists(dir));
+    at.touch(DIR_FILE);
 
-    at.touch(file);
+    ucmd.arg(DIR)
-    assert!(at.file_exists(file));
+        .fails()
+        .stderr_is(format!("rmdir: failed to remove 'dir': {}", NOT_EMPTY));
 
-    ucmd.arg(dir).fails().stderr_is(
+    assert!(at.dir_exists(DIR));
-        "rmdir: failed to remove 'test_rmdir_nonempty_no_parents': Directory not \
-         empty\n",
-    );
-
-    assert!(at.dir_exists(dir));
 }
 
 #[test]
 fn test_rmdir_nonempty_directory_with_parents() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let dir = "test_rmdir_nonempty/with/parents";
-    let file = "test_rmdir_nonempty/with/parents/foo";
 
-    at.mkdir_all(dir);
+    at.mkdir_all(NESTED_DIR);
-    assert!(at.dir_exists(dir));
+    at.touch(NESTED_DIR_FILE);
 
-    at.touch(file);
+    ucmd.arg("-p").arg(NESTED_DIR).fails().stderr_is(format!(
-    assert!(at.file_exists(file));
+        "rmdir: failed to remove 'dir/ect/ory': {}",
+        NOT_EMPTY
+    ));
 
-    ucmd.arg("-p").arg(dir).fails().stderr_is(
+    assert!(at.dir_exists(NESTED_DIR));
-        "rmdir: failed to remove 'test_rmdir_nonempty/with/parents': Directory not \
-         empty\nrmdir: failed to remove 'test_rmdir_nonempty/with': Directory not \
-         empty\nrmdir: failed to remove 'test_rmdir_nonempty': Directory not \
-         empty\n",
-    );
-
-    assert!(at.dir_exists(dir));
 }
 
 #[test]
 fn test_rmdir_ignore_nonempty_directory_no_parents() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let dir = "test_rmdir_ignore_nonempty_no_parents";
-    let file = "test_rmdir_ignore_nonempty_no_parents/foo";
 
-    at.mkdir(dir);
+    at.mkdir(DIR);
-    assert!(at.dir_exists(dir));
+    at.touch(DIR_FILE);
-
-    at.touch(file);
-    assert!(at.file_exists(file));
 
     ucmd.arg("--ignore-fail-on-non-empty")
-        .arg(dir)
+        .arg(DIR)
         .succeeds()
         .no_stderr();
 
-    assert!(at.dir_exists(dir));
+    assert!(at.dir_exists(DIR));
 }
 
 #[test]
 fn test_rmdir_ignore_nonempty_directory_with_parents() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let dir = "test_rmdir_ignore_nonempty/with/parents";
-    let file = "test_rmdir_ignore_nonempty/with/parents/foo";
 
-    at.mkdir_all(dir);
+    at.mkdir_all(NESTED_DIR);
-    assert!(at.dir_exists(dir));
+    at.touch(NESTED_DIR_FILE);
-
-    at.touch(file);
-    assert!(at.file_exists(file));
 
     ucmd.arg("--ignore-fail-on-non-empty")
         .arg("-p")
-        .arg(dir)
+        .arg(NESTED_DIR)
         .succeeds()
         .no_stderr();
 
-    assert!(at.dir_exists(dir));
+    assert!(at.dir_exists(NESTED_DIR));
 }
 
 #[test]
-fn test_rmdir_remove_symlink_match_gnu_error() {
+fn test_rmdir_not_a_directory() {
     let (at, mut ucmd) = at_and_ucmd!();
 
-    let file = "file";
+    at.touch("file");
-    let fl = "fl";
-    at.touch(file);
-    assert!(at.file_exists(file));
-    at.symlink_file(file, fl);
-    assert!(at.file_exists(fl));
 
-    ucmd.arg("fl/")
+    ucmd.arg("--ignore-fail-on-non-empty")
+        .arg("file")
         .fails()
-        .stderr_is("rmdir: failed to remove 'fl/': Not a directory");
+        .no_stdout()
+        .stderr_is(format!(
+            "rmdir: failed to remove 'file': {}",
+            NOT_A_DIRECTORY
+        ));
+}
+
+#[test]
+fn test_verbose_single() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.mkdir(DIR);
+
+    ucmd.arg("-v")
+        .arg(DIR)
+        .succeeds()
+        .no_stderr()
+        .stdout_is("rmdir: removing directory, 'dir'\n");
+}
+
+#[test]
+fn test_verbose_multi() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.mkdir(DIR);
+
+    ucmd.arg("-v")
+        .arg("does_not_exist")
+        .arg(DIR)
+        .fails()
+        .stdout_is(
+            "rmdir: removing directory, 'does_not_exist'\n\
+             rmdir: removing directory, 'dir'\n",
+        )
+        .stderr_is(format!(
+            "rmdir: failed to remove 'does_not_exist': {}",
+            NOT_FOUND
+        ));
+}
+
+#[test]
+fn test_verbose_nested_failure() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.mkdir_all(NESTED_DIR);
+    at.touch("dir/ect/file");
+
+    ucmd.arg("-pv")
+        .arg(NESTED_DIR)
+        .fails()
+        .stdout_is(
+            "rmdir: removing directory, 'dir/ect/ory'\n\
+             rmdir: removing directory, 'dir/ect'\n",
+        )
+        .stderr_is(format!("rmdir: failed to remove 'dir/ect': {}", NOT_EMPTY));
+}
+
+#[cfg(unix)]
+#[test]
+fn test_rmdir_ignore_nonempty_no_permissions() {
+    use std::fs;
+
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    // We make the *parent* dir read-only to prevent deleting the dir in it.
+    at.mkdir_all("dir/ect/ory");
+    at.touch("dir/ect/ory/file");
+    let dir_ect = at.plus("dir/ect");
+    let mut perms = fs::metadata(&dir_ect).unwrap().permissions();
+    perms.set_readonly(true);
+    fs::set_permissions(&dir_ect, perms.clone()).unwrap();
+
+    // rmdir should now get a permissions error that it interprets as
+    // a non-empty error.
+    ucmd.arg("--ignore-fail-on-non-empty")
+        .arg("dir/ect/ory")
+        .succeeds()
+        .no_stderr();
+
+    assert!(at.dir_exists("dir/ect/ory"));
+
+    // Politely restore permissions for cleanup
+    perms.set_readonly(false);
+    fs::set_permissions(&dir_ect, perms).unwrap();
+}
+
+#[test]
+fn test_rmdir_remove_symlink_file() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.touch("file");
+    at.symlink_file("file", "fl");
+
+    ucmd.arg("fl/").fails().stderr_is(format!(
+        "rmdir: failed to remove 'fl/': {}",
+        NOT_A_DIRECTORY
+    ));
+}
+
+// This behavior is known to happen on Linux but not all Unixes
+#[cfg(any(target_os = "linux", target_os = "android"))]
+#[test]
+fn test_rmdir_remove_symlink_dir() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.mkdir("dir");
+    at.symlink_dir("dir", "dl");
+
+    ucmd.arg("dl/")
+        .fails()
+        .stderr_is("rmdir: failed to remove 'dl/': Symbolic link not followed");
+}
+
+#[cfg(any(target_os = "linux", target_os = "android"))]
+#[test]
+fn test_rmdir_remove_symlink_dangling() {
+    let (at, mut ucmd) = at_and_ucmd!();
+
+    at.symlink_dir("dir", "dl");
+
+    ucmd.arg("dl/")
+        .fails()
+        .stderr_is("rmdir: failed to remove 'dl/': Symbolic link not followed");
 }

tests/by-util/test_wc.rs

@@ -1,6 +1,6 @@
 use crate::common::util::*;
 
-// spell-checker:ignore (flags) lwmcL clmwL ; (path) bogusfile emptyfile manyemptylines moby notrailingnewline onelongemptyline onelongword
+// spell-checker:ignore (flags) lwmcL clmwL ; (path) bogusfile emptyfile manyemptylines moby notrailingnewline onelongemptyline onelongword weirdchars
 
 #[test]
 fn test_count_bytes_large_stdin() {
@@ -53,11 +53,16 @@ fn test_utf8() {
         .args(&["-lwmcL"])
         .pipe_in_fixture("UTF_8_test.txt")
         .run()
-        .stdout_is("    300    4969   22781   22213      79\n");
+        .stdout_is("    303    2119   22457   23025      79\n");
-    // GNU returns "    300    2086   22219   22781      79"
+}
-    //
+
-    // TODO: we should fix the word, character, and byte count to
+#[test]
-    // match the behavior of GNU wc
+fn test_utf8_extra() {
+    new_ucmd!()
+        .arg("-lwmcL")
+        .pipe_in_fixture("UTF_8_weirdchars.txt")
+        .run()
+        .stdout_is("     25      87     442     513      48\n");
 }
 
 #[test]
@@ -200,22 +205,33 @@ fn test_file_bytes_dictate_width() {
 /// Test that getting counts from a directory is an error.
 #[test]
 fn test_read_from_directory_error() {
-    // TODO To match GNU `wc`, the `stdout` should be:
+    #[cfg(not(windows))]
-    //
+    const STDERR: &str = ".: Is a directory";
-    //     "      0       0       0 .\n"
+    #[cfg(windows)]
-    //
+    const STDERR: &str = ".: Access is denied";
+
+    #[cfg(not(windows))]
+    const STDOUT: &str = "      0       0       0 .\n";
+    #[cfg(windows)]
+    const STDOUT: &str = "";
+
     new_ucmd!()
         .args(&["."])
         .fails()
-        .stderr_contains(".: Is a directory\n")
+        .stderr_contains(STDERR)
-        .stdout_is("0 0 0 .\n");
+        .stdout_is(STDOUT);
 }
 
 /// Test that getting counts from nonexistent file is an error.
 #[test]
 fn test_read_from_nonexistent_file() {
+    #[cfg(not(windows))]
+    const MSG: &str = "bogusfile: No such file or directory";
+    #[cfg(windows)]
+    const MSG: &str = "bogusfile: The system cannot find the file specified";
     new_ucmd!()
         .args(&["bogusfile"])
         .fails()
-        .stderr_contains("bogusfile: No such file or directory\n");
+        .stderr_contains(MSG)
+        .stdout_is("");
 }

tests/fixtures/wc/UTF_8_weirdchars.txt

@@ -0,0 +1,25 @@
+zero-width space inbetween these: x​x
+and inbetween two spaces: [ ​ ]
+and at the end of the line: ​
+
+non-breaking space: x x [   ]  
+
+simple unicode: xµx [ µ ] µ
+
+wide: xｗx [ ｗ ] ｗ
+
+simple emoji: x👩x [ 👩 ] 👩
+
+complex emoji: x👩‍🔬x [ 👩‍🔬 ] 👩‍🔬
+
+Ｈｅｌｌｏ, ｗｏｒｌｄ!
+
+line feed: xx [  ] 
+
+vertical tab: xx [  ] 
+
+horizontal tab: x	x [ 	 ]
+this should be the longest line:
+1234567	12345678	123456781234567812345678
+
+Control character: xx [  ] 

util/run-gnu-test.sh

@@ -1,5 +1,6 @@
 #!/bin/bash
 # spell-checker:ignore (env/vars) BUILDDIR GNULIB SUBDIRS
+cd "$(dirname "${BASH_SOURCE[0]}")/../.."
 set -e
 BUILDDIR="${PWD}/uutils/target/release"
 GNULIB_DIR="${PWD}/gnulib"