Merge branch 'master' into chgrp/no-err

.github/workflows/GnuTests.yml

@@ -90,3 +90,42 @@ jobs:
       with:
         name: gnu-result
         path: gnu-result.json
+    - name: Download the result
+      uses: dawidd6/action-download-artifact@v2
+      with:
+        workflow: GnuTests.yml
+        name: gnu-result
+        repo: uutils/coreutils
+        branch: master
+        path: dl
+    - name: Download the log
+      uses: dawidd6/action-download-artifact@v2
+      with:
+        workflow: GnuTests.yml
+        name: test-report
+        repo: uutils/coreutils
+        branch: master
+        path: dl
+    - name: Compare failing tests against master
+      shell: bash
+      run: |
+            OLD_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" dl/test-suite.log | sort)
+            NEW_FAILING=$(sed -n "s/^FAIL: \([[:print:]]\+\).*/\1/p" gnu/tests/test-suite.log | sort)
+            for LINE in $OLD_FAILING
+            do
+              if ! grep -Fxq $LINE<<<"$NEW_FAILING"; then
+                echo "::warning ::Congrats! The gnu test $LINE is now passing!"
+              fi
+            done
+            for LINE in $NEW_FAILING
+            do
+              if ! grep -Fxq $LINE<<<"$OLD_FAILING"
+              then
+                echo "::error ::GNU test failed: $LINE. $LINE is passing on 'master'. Maybe you have to rebase?"
+              fi
+            done
+    - name: Compare against master results
+      shell: bash
+      run: |
+        mv dl/gnu-result.json master-gnu-result.json
+        python uutils/util/compare_gnu_result.py

Cargo.lock

@@ -713,9 +713,9 @@ checksum = "31a7a908b8f32538a2143e59a6e4e2508988832d5d4d6f7c156b3cbc762643a5"
 
 [[package]]
 name = "filetime"
-version = "0.2.14"
+version = "0.2.15"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1d34cfa13a63ae058bfa601fe9e313bbdb3746427c1459185464ce0fcf62e1e8"
+checksum = "975ccf83d8d9d0d84682850a38c8169027be83368805971cc4f238c2b245bc98"
 dependencies = [
  "cfg-if 1.0.0",
  "libc",
@@ -1631,9 +1631,9 @@ checksum = "d29ab0c6d3fc0ee92fe66e2d99f700eab17a8d57d1c1d3b748380fb20baa78cd"
 
 [[package]]
 name = "selinux"
-version = "0.1.3"
+version = "0.2.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "bd525eeb189eb26c8471463186bba87644e3d8a9c7ae392adaf9ec45ede574bc"
+checksum = "1aa2f705dd871c2eb90888bb2d44b13218b34f5c7318c3971df62f799d0143eb"
 dependencies = [
  "bitflags",
  "libc",
@@ -2023,6 +2023,7 @@ dependencies = [
  "unix_socket",
  "uucore",
  "uucore_procs",
+ "winapi-util",
 ]
 
 [[package]]
@@ -2032,7 +2033,7 @@ dependencies = [
  "clap",
  "fts-sys",
  "libc",
- "selinux-sys",
+ "selinux",
  "thiserror",
  "uucore",
  "uucore_procs",

Cargo.toml

@@ -241,7 +241,7 @@ clap = { version = "2.33", features = ["wrap_help"] }
 lazy_static = { version="1.3" }
 textwrap = { version="=0.11.0", features=["term_size"] } # !maint: [2020-05-10; rivy] unstable crate using undocumented features; pinned currently, will review
 uucore = { version=">=0.0.9", package="uucore", path="src/uucore" }
-selinux = { version="0.1.3", optional = true }
+selinux = { version="0.2.1", optional = true }
 # * uutils
 uu_test  = { optional=true, version="0.0.7", package="uu_test", path="src/uu/test" }
 #

README.md

@@ -365,25 +365,24 @@ To contribute to uutils, please see [CONTRIBUTING](CONTRIBUTING.md).
 
 | Done      | Semi-Done | To Do  |
 |-----------|-----------|--------|
-| arch      | cp        | chcon  |
+| arch      | cp        | runcon |
-| base32    | date      | runcon |
+| base32    | date      | stty   |
-| base64    | dd        | stty   |
+| base64    | dd        |        |
 | basename  | df        |        |
 | basenc    | expr      |        |
 | cat       | install   |        |
 | chcon     | join      |        |
-| cat       | ls        |        |
+| chgrp     | ls        |        |
-| chgrp     | more      |        |
+| chmod     | more      |        |
-| chmod     | numfmt    |        |
+| chown     | numfmt    |        |
-| chown     | od (`--strings` and 128-bit data types missing) | |
+| chroot    | od (`--strings` and 128-bit data types missing) | |
-| chroot    | pr        |        |
+| cksum     | pr        |        |
-| cksum     | printf    |        |
+| comm      | printf    |        |
-| comm      | sort      |        |
+| csplit    | sort      |        |
-| csplit    | split     |        |
+| cut       | split     |        |
-| cut       | tac       |        |
+| dircolors | tac       |        |
-| dircolors | tail      |        |
+| dirname   | tail      |        |
-| dirname   | test      |        |
+| du        | test      |        |
-| du        |           |        |
 | echo      |           |        |
 | env       |           |        |
 | expand    |           |        |

src/uu/cat/Cargo.toml

@@ -23,9 +23,10 @@ uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_p
 
 [target.'cfg(unix)'.dependencies]
 unix_socket = "0.5.0"
+nix = "0.20.0"
 
-[target.'cfg(any(target_os = "linux", target_os = "android"))'.dependencies]
+[target.'cfg(windows)'.dependencies]
-nix = "0.20"
+winapi-util = "0.1.5"
 
 [[bin]]
 name = "cat"

src/uu/cat/src/cat.rs

@@ -22,11 +22,14 @@ use std::io::{self, Read, Write};
 use thiserror::Error;
 use uucore::error::UResult;
 
+#[cfg(unix)]
+use std::os::unix::io::AsRawFd;
+
 /// Linux splice support
 #[cfg(any(target_os = "linux", target_os = "android"))]
 mod splice;
 #[cfg(any(target_os = "linux", target_os = "android"))]
-use std::os::unix::io::{AsRawFd, RawFd};
+use std::os::unix::io::RawFd;
 
 /// Unix domain socket support
 #[cfg(unix)]
@@ -59,6 +62,8 @@ enum CatError {
     },
     #[error("Is a directory")]
     IsDirectory,
+    #[error("input file is output file")]
+    OutputIsInput,
 }
 
 type CatResult<T> = Result<T, CatError>;
@@ -123,6 +128,12 @@ struct OutputState {
 
     /// Whether the output cursor is at the beginning of a new line
     at_line_start: bool,
+
+    /// Whether we skipped a \r, which still needs to be printed
+    skipped_carriage_return: bool,
+
+    /// Whether we have already printed a blank line
+    one_blank_kept: bool,
 }
 
 /// Represents an open file handle, stream, or other device
@@ -297,7 +308,13 @@ fn cat_handle<R: Read>(
     }
 }
 
-fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> CatResult<()> {
+fn cat_path(
+    path: &str,
+    options: &OutputOptions,
+    state: &mut OutputState,
+    #[cfg(unix)] out_info: &nix::sys::stat::FileStat,
+    #[cfg(windows)] out_info: &winapi_util::file::Information,
+) -> CatResult<()> {
     if path == "-" {
         let stdin = io::stdin();
         let mut handle = InputHandle {
@@ -324,6 +341,10 @@ fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> Cat
         }
         _ => {
             let file = File::open(path)?;
+            #[cfg(any(windows, unix))]
+            if same_file(out_info, &file) {
+                return Err(CatError::OutputIsInput);
+            }
             let mut handle = InputHandle {
                 #[cfg(any(target_os = "linux", target_os = "android"))]
                 file_descriptor: file.as_raw_fd(),
@@ -335,18 +356,42 @@ fn cat_path(path: &str, options: &OutputOptions, state: &mut OutputState) -> Cat
     }
 }
 
+#[cfg(unix)]
+fn same_file(a_info: &nix::sys::stat::FileStat, b: &File) -> bool {
+    let b_info = nix::sys::stat::fstat(b.as_raw_fd()).unwrap();
+    b_info.st_size != 0 && b_info.st_dev == a_info.st_dev && b_info.st_ino == a_info.st_ino
+}
+
+#[cfg(windows)]
+fn same_file(a_info: &winapi_util::file::Information, b: &File) -> bool {
+    let b_info = winapi_util::file::information(b).unwrap();
+    b_info.file_size() != 0
+        && b_info.volume_serial_number() == a_info.volume_serial_number()
+        && b_info.file_index() == a_info.file_index()
+}
+
 fn cat_files(files: Vec<String>, options: &OutputOptions) -> UResult<()> {
+    #[cfg(windows)]
+    let out_info = winapi_util::file::information(&std::io::stdout()).unwrap();
+    #[cfg(unix)]
+    let out_info = nix::sys::stat::fstat(std::io::stdout().as_raw_fd()).unwrap();
+
     let mut state = OutputState {
         line_number: 1,
         at_line_start: true,
+        skipped_carriage_return: false,
+        one_blank_kept: false,
     };
     let mut error_messages: Vec<String> = Vec::new();
 
     for path in &files {
-        if let Err(err) = cat_path(path, options, &mut state) {
+        if let Err(err) = cat_path(path, options, &mut state, &out_info) {
             error_messages.push(format!("{}: {}", path, err));
         }
     }
+    if state.skipped_carriage_return {
+        print!("\r");
+    }
     if error_messages.is_empty() {
         Ok(())
     } else {
@@ -424,7 +469,6 @@ fn write_lines<R: Read>(
     let mut in_buf = [0; 1024 * 31];
     let stdout = io::stdout();
     let mut writer = stdout.lock();
-    let mut one_blank_kept = false;
 
     while let Ok(n) = handle.reader.read(&mut in_buf) {
         if n == 0 {
@@ -435,8 +479,13 @@ fn write_lines<R: Read>(
         while pos < n {
             // skip empty line_number enumerating them if needed
             if in_buf[pos] == b'\n' {
-                if !state.at_line_start || !options.squeeze_blank || !one_blank_kept {
+                // \r followed by \n is printed as ^M when show_ends is enabled, so that \r\n prints as ^M$
-                    one_blank_kept = true;
+                if state.skipped_carriage_return && options.show_ends {
+                    writer.write_all(b"^M")?;
+                    state.skipped_carriage_return = false;
+                }
+                if !state.at_line_start || !options.squeeze_blank || !state.one_blank_kept {
+                    state.one_blank_kept = true;
                     if state.at_line_start && options.number == NumberingMode::All {
                         write!(&mut writer, "{0:6}\t", state.line_number)?;
                         state.line_number += 1;
@@ -450,7 +499,12 @@ fn write_lines<R: Read>(
                 pos += 1;
                 continue;
             }
-            one_blank_kept = false;
+            if state.skipped_carriage_return {
+                writer.write_all(b"\r")?;
+                state.skipped_carriage_return = false;
+                state.at_line_start = false;
+            }
+            state.one_blank_kept = false;
             if state.at_line_start && options.number != NumberingMode::None {
                 write!(&mut writer, "{0:6}\t", state.line_number)?;
                 state.line_number += 1;
@@ -465,17 +519,22 @@ fn write_lines<R: Read>(
                 write_to_end(&in_buf[pos..], &mut writer)
             };
             // end of buffer?
-            if offset == 0 {
+            if offset + pos == in_buf.len() {
                 state.at_line_start = false;
                 break;
             }
+            if in_buf[pos + offset] == b'\r' {
+                state.skipped_carriage_return = true;
+            } else {
+                assert_eq!(in_buf[pos + offset], b'\n');
                 // print suitable end of line
                 writer.write_all(options.end_of_line().as_bytes())?;
                 if handle.is_interactive {
                     writer.flush()?;
                 }
                 state.at_line_start = true;
-            pos += offset;
+            }
+            pos += offset + 1;
         }
     }
 
@@ -483,17 +542,19 @@ fn write_lines<R: Read>(
 }
 
 // write***_to_end methods
-// Write all symbols till end of line or end of buffer is reached
+// Write all symbols till \n or \r or end of buffer is reached
-// Return the (number of written symbols + 1) or 0 if the end of buffer is reached
+// We need to stop at \r because it may be written as ^M depending on the byte after and settings;
+// however, write_nonprint_to_end doesn't need to stop at \r because it will always write \r as ^M.
+// Return the number of written symbols
 fn write_to_end<W: Write>(in_buf: &[u8], writer: &mut W) -> usize {
-    match in_buf.iter().position(|c| *c == b'\n') {
+    match in_buf.iter().position(|c| *c == b'\n' || *c == b'\r') {
         Some(p) => {
             writer.write_all(&in_buf[..p]).unwrap();
-            p + 1
+            p
         }
         None => {
             writer.write_all(in_buf).unwrap();
-            0
+            in_buf.len()
         }
     }
 }
@@ -501,20 +562,25 @@ fn write_to_end<W: Write>(in_buf: &[u8], writer: &mut W) -> usize {
 fn write_tab_to_end<W: Write>(mut in_buf: &[u8], writer: &mut W) -> usize {
     let mut count = 0;
     loop {
-        match in_buf.iter().position(|c| *c == b'\n' || *c == b'\t') {
+        match in_buf
+            .iter()
+            .position(|c| *c == b'\n' || *c == b'\t' || *c == b'\r')
+        {
             Some(p) => {
                 writer.write_all(&in_buf[..p]).unwrap();
                 if in_buf[p] == b'\n' {
-                    return count + p + 1;
+                    return count + p;
-                } else {
+                } else if in_buf[p] == b'\t' {
                     writer.write_all(b"^I").unwrap();
                     in_buf = &in_buf[p + 1..];
                     count += p + 1;
+                } else {
+                    return count + p;
                 }
             }
             None => {
                 writer.write_all(in_buf).unwrap();
-                return 0;
+                return in_buf.len();
             }
         };
     }
@@ -539,11 +605,7 @@ fn write_nonprint_to_end<W: Write>(in_buf: &[u8], writer: &mut W, tab: &[u8]) ->
         .unwrap();
         count += 1;
     }
-    if count != in_buf.len() {
+    count
-        count + 1
-    } else {
-        0
-    }
 }
 
 #[cfg(test)]

src/uu/chcon/Cargo.toml

@@ -17,7 +17,7 @@ path = "src/chcon.rs"
 clap         = { version = "2.33", features = ["wrap_help"] }
 uucore       = { version = ">=0.0.9", package="uucore", path="../../uucore", features=["entries", "fs", "perms"] }
 uucore_procs = { version = ">=0.0.6", package="uucore_procs", path="../../uucore_procs" }
-selinux-sys  = { version = "0.5"   }
+selinux      = { version = "0.2"   }
 fts-sys      = { version = "0.2"   }
 thiserror    = { version = "1.0"   }
 libc         = { version = "0.2"   }

src/uu/chcon/src/chcon.rs

@@ -5,14 +5,18 @@
 use uucore::{executable, show_error, show_usage_error, show_warning};
 
 use clap::{App, Arg};
+use selinux::{OpaqueSecurityContext, SecurityContext};
 
+use std::borrow::Cow;
 use std::ffi::{CStr, CString, OsStr, OsString};
-use std::fmt::Write;
+use std::os::raw::c_int;
-use std::os::raw::{c_char, c_int};
 use std::path::{Path, PathBuf};
-use std::{fs, io, ptr, slice};
+use std::{fs, io};
 
-type Result<T> = std::result::Result<T, Error>;
+mod errors;
+mod fts;
+
+use errors::*;
 
 static VERSION: &str = env!("CARGO_PKG_VERSION");
 static ABOUT: &str = "Change the SELinux security context of each FILE to CONTEXT. \n\
@@ -81,15 +85,16 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
     let context = match &options.mode {
         CommandLineMode::ReferenceBased { reference } => {
-            let result = selinux::FileContext::new(reference, true)
+            let result = match SecurityContext::of_path(reference, true, false) {
-                .and_then(|r| {
+                Ok(Some(context)) => Ok(context),
-                    if r.is_empty() {
+
-                        Err(io::Error::from_raw_os_error(libc::ENODATA))
+                Ok(None) => {
-                    } else {
+                    let err = io::Error::from_raw_os_error(libc::ENODATA);
-                        Ok(r)
+                    Err(Error::from_io1("Getting security context", reference, err))
                 }
-                })
+
-                .map_err(|r| Error::io1("Getting security context", reference, r));
+                Err(r) => Err(Error::from_selinux("Getting security context", r)),
+            };
 
             match result {
                 Err(r) => {
@@ -102,33 +107,24 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         }
 
         CommandLineMode::ContextBased { context } => {
-            match selinux::SecurityContext::security_check_context(context)
+            let c_context = match os_str_to_c_string(context) {
-                .map_err(|r| Error::io1("Checking security context", context, r))
+                Ok(context) => context,
-            {
-                Err(r) => {
-                    show_error!("{}.", report_full_error(&r));
-                    return libc::EXIT_FAILURE;
-                }
 
-                Ok(Some(false)) => {
+                Err(_r) => {
                     show_error!("Invalid security context '{}'.", context.to_string_lossy());
                     return libc::EXIT_FAILURE;
                 }
-
-                Ok(Some(true)) | Ok(None) => {}
-            }
-
-            let c_context = if let Ok(value) = os_str_to_c_string(context) {
-                value
-            } else {
-                show_error!("Invalid security context '{}'.", context.to_string_lossy());
-                return libc::EXIT_FAILURE;
             };
 
-            SELinuxSecurityContext::String(c_context)
+            if SecurityContext::from_c_str(&c_context, false).check() == Some(false) {
+                show_error!("Invalid security context '{}'.", context.to_string_lossy());
+                return libc::EXIT_FAILURE;
             }
 
-        CommandLineMode::Custom { .. } => SELinuxSecurityContext::default(),
+            SELinuxSecurityContext::String(Some(c_context))
+        }
+
+        CommandLineMode::Custom { .. } => SELinuxSecurityContext::String(None),
     };
 
     let root_dev_ino = if options.preserve_root && options.recursive_mode.is_recursive() {
@@ -282,65 +278,6 @@ pub fn uu_app() -> App<'static, 'static> {
         .arg(Arg::with_name("FILE").multiple(true).min_values(1))
 }
 
-fn report_full_error(mut err: &dyn std::error::Error) -> String {
-    let mut desc = String::with_capacity(256);
-    write!(&mut desc, "{}", err).unwrap();
-    while let Some(source) = err.source() {
-        err = source;
-        write!(&mut desc, ". {}", err).unwrap();
-    }
-    desc
-}
-
-#[derive(thiserror::Error, Debug)]
-enum Error {
-    #[error("No context is specified")]
-    MissingContext,
-
-    #[error("No files are specified")]
-    MissingFiles,
-
-    #[error("{0}")]
-    ArgumentsMismatch(String),
-
-    #[error(transparent)]
-    CommandLine(#[from] clap::Error),
-
-    #[error("{operation} failed")]
-    Io {
-        operation: &'static str,
-        source: io::Error,
-    },
-
-    #[error("{operation} failed on '{}'", .operand1.to_string_lossy())]
-    Io1 {
-        operation: &'static str,
-        operand1: OsString,
-        source: io::Error,
-    },
-}
-
-impl Error {
-    fn io1(operation: &'static str, operand1: impl Into<OsString>, source: io::Error) -> Self {
-        Self::Io1 {
-            operation,
-            operand1: operand1.into(),
-            source,
-        }
-    }
-
-    #[cfg(unix)]
-    fn io1_c_str(operation: &'static str, operand1: &CStr, source: io::Error) -> Self {
-        if operand1.to_bytes().is_empty() {
-            Self::Io { operation, source }
-        } else {
-            use std::os::unix::ffi::OsStrExt;
-
-            Self::io1(operation, OsStr::from_bytes(operand1.to_bytes()), source)
-        }
-    }
-}
-
 #[derive(Debug)]
 struct Options {
     verbose: bool,
@@ -500,39 +437,29 @@ fn process_files(
     root_dev_ino: Option<(libc::ino_t, libc::dev_t)>,
 ) -> Vec<Error> {
     let fts_options = options.recursive_mode.fts_open_options();
-    let mut fts = match fts::FTS::new(options.files.iter(), fts_options, None) {
+    let mut fts = match fts::FTS::new(options.files.iter(), fts_options) {
         Ok(fts) => fts,
-        Err(source) => {
+        Err(err) => return vec![err],
-            return vec![Error::Io {
-                operation: "fts_open()",
-                source,
-            }]
-        }
     };
 
-    let mut results = vec![];
+    let mut errors = Vec::default();
-
     loop {
         match fts.read_next_entry() {
             Ok(true) => {
                 if let Err(err) = process_file(options, context, &mut fts, root_dev_ino) {
-                    results.push(err);
+                    errors.push(err);
                 }
             }
 
             Ok(false) => break,
 
-            Err(source) => {
+            Err(err) => {
-                results.push(Error::Io {
+                errors.push(err);
-                    operation: "fts_read()",
-                    source,
-                });
-
                 break;
             }
         }
     }
-    results
+    errors
 }
 
 fn process_file(
@@ -541,46 +468,40 @@ fn process_file(
     fts: &mut fts::FTS,
     root_dev_ino: Option<(libc::ino_t, libc::dev_t)>,
 ) -> Result<()> {
-    let entry = fts.last_entry_mut().unwrap();
+    let mut entry = fts.last_entry_ref().unwrap();
 
-    let file_full_name = if entry.fts_path.is_null() {
+    let file_full_name = entry.path().map(PathBuf::from).ok_or_else(|| {
-        None
+        Error::from_io("File name validation", io::ErrorKind::InvalidInput.into())
-    } else {
-        let fts_path_size = usize::from(entry.fts_pathlen).saturating_add(1);
-
-        // SAFETY: `entry.fts_path` is a non-null pointer that is assumed to be valid.
-        let bytes = unsafe { slice::from_raw_parts(entry.fts_path.cast(), fts_path_size) };
-        CStr::from_bytes_with_nul(bytes).ok()
-    }
-    .ok_or_else(|| Error::Io {
-        operation: "File name validation",
-        source: io::ErrorKind::InvalidInput.into(),
     })?;
 
-    let fts_access_path = ptr_to_c_str(entry.fts_accpath)
+    let fts_access_path = entry.access_path().ok_or_else(|| {
-        .map_err(|r| Error::io1_c_str("File name validation", file_full_name, r))?;
+        let err = io::ErrorKind::InvalidInput.into();
+        Error::from_io1("File name validation", &file_full_name, err)
+    })?;
 
-    let err = |s, k: io::ErrorKind| Error::io1_c_str(s, file_full_name, k.into());
+    let err = |s, k: io::ErrorKind| Error::from_io1(s, &file_full_name, k.into());
 
     let fts_err = |s| {
-        let r = io::Error::from_raw_os_error(entry.fts_errno);
+        let r = io::Error::from_raw_os_error(entry.errno());
-        Err(Error::io1_c_str(s, file_full_name, r))
+        Err(Error::from_io1(s, &file_full_name, r))
     };
 
     // SAFETY: If `entry.fts_statp` is not null, then is is assumed to be valid.
-    let file_dev_ino = unsafe { entry.fts_statp.as_ref() }
+    let file_dev_ino = if let Some(stat) = entry.stat() {
-        .map(|stat| (stat.st_ino, stat.st_dev))
+        (stat.st_ino, stat.st_dev)
-        .ok_or_else(|| err("Getting meta data", io::ErrorKind::InvalidInput))?;
+    } else {
+        return Err(err("Getting meta data", io::ErrorKind::InvalidInput));
+    };
 
     let mut result = Ok(());
 
-    match c_int::from(entry.fts_info) {
+    match entry.flags() {
         fts_sys::FTS_D => {
             if options.recursive_mode.is_recursive() {
                 if root_dev_ino_check(root_dev_ino, file_dev_ino) {
                     // This happens e.g., with "chcon -R --preserve-root ... /"
                     // and with "chcon -RH --preserve-root ... symlink-to-root".
-                    root_dev_ino_warn(file_full_name);
+                    root_dev_ino_warn(&file_full_name);
 
                     // Tell fts not to traverse into this hierarchy.
                     let _ignored = fts.set(fts_sys::FTS_SKIP);
@@ -607,8 +528,8 @@ fn process_file(
             // that modify permissions, it is possible that the file in question is accessible when
             // control reaches this point. So, if this is the first time we've seen the FTS_NS for
             // this file, tell fts_read to stat it "again".
-            if entry.fts_level == 0 && entry.fts_number == 0 {
+            if entry.level() == 0 && entry.number() == 0 {
-                entry.fts_number = 1;
+                entry.set_number(1);
                 let _ignored = fts.set(fts_sys::FTS_AGAIN);
                 return Ok(());
             }
@@ -621,8 +542,8 @@ fn process_file(
         fts_sys::FTS_DNR => result = fts_err("Reading directory"),
 
         fts_sys::FTS_DC => {
-            if cycle_warning_required(options.recursive_mode.fts_open_options(), entry) {
+            if cycle_warning_required(options.recursive_mode.fts_open_options(), &entry) {
-                emit_cycle_warning(file_full_name);
+                emit_cycle_warning(&file_full_name);
                 return Err(err("Reading cyclic directory", io::ErrorKind::InvalidData));
             }
         }
@@ -630,11 +551,11 @@ fn process_file(
         _ => {}
     }
 
-    if c_int::from(entry.fts_info) == fts_sys::FTS_DP
+    if entry.flags() == fts_sys::FTS_DP
         && result.is_ok()
         && root_dev_ino_check(root_dev_ino, file_dev_ino)
     {
-        root_dev_ino_warn(file_full_name);
+        root_dev_ino_warn(&file_full_name);
         result = Err(err("Modifying root path", io::ErrorKind::PermissionDenied));
     }
 
@@ -656,24 +577,10 @@ fn process_file(
     result
 }
 
-fn set_file_security_context(
-    path: &Path,
-    context: *const c_char,
-    follow_symbolic_links: bool,
-) -> Result<()> {
-    let mut file_context = selinux::FileContext::from_ptr(context as *mut c_char);
-    if file_context.context.is_null() {
-        Err(io::Error::from(io::ErrorKind::InvalidInput))
-    } else {
-        file_context.set_for_file(path, follow_symbolic_links)
-    }
-    .map_err(|r| Error::io1("Setting security context", path, r))
-}
-
 fn change_file_context(
     options: &Options,
     context: &SELinuxSecurityContext,
-    file: &CStr,
+    path: &Path,
 ) -> Result<()> {
     match &options.mode {
         CommandLineMode::Custom {
@@ -682,91 +589,88 @@ fn change_file_context(
             the_type,
             range,
         } => {
-            let path = PathBuf::from(c_str_to_os_string(file));
+            let err0 = || -> Result<()> {
-            let file_context = selinux::FileContext::new(&path, options.affect_symlink_referent)
-                .map_err(|r| Error::io1("Getting security context", &path, r))?;
-
                 // If the file doesn't have a context, and we're not setting all of the context
                 // components, there isn't really an obvious default. Thus, we just give up.
-            if file_context.is_empty() {
+                let op = "Applying partial security context to unlabeled file";
-                return Err(Error::io1(
+                let err = io::ErrorKind::InvalidInput.into();
-                    "Applying partial security context to unlabeled file",
+                Err(Error::from_io1(op, path, err))
-                    path,
+            };
-                    io::ErrorKind::InvalidInput.into(),
+
-                ));
+            let file_context =
+                match SecurityContext::of_path(path, options.affect_symlink_referent, false) {
+                    Ok(Some(context)) => context,
+
+                    Ok(None) => return err0(),
+                    Err(r) => return Err(Error::from_selinux("Getting security context", r)),
+                };
+
+            let c_file_context = match file_context.to_c_string() {
+                Ok(Some(context)) => context,
+
+                Ok(None) => return err0(),
+                Err(r) => return Err(Error::from_selinux("Getting security context", r)),
+            };
+
+            let se_context =
+                OpaqueSecurityContext::from_c_str(c_file_context.as_ref()).map_err(|_r| {
+                    let err = io::ErrorKind::InvalidInput.into();
+                    Error::from_io1("Creating security context", path, err)
+                })?;
+
+            type SetValueProc = fn(&OpaqueSecurityContext, &CStr) -> selinux::errors::Result<()>;
+
+            let list: &[(&Option<OsString>, SetValueProc)] = &[
+                (user, OpaqueSecurityContext::set_user),
+                (role, OpaqueSecurityContext::set_role),
+                (the_type, OpaqueSecurityContext::set_type),
+                (range, OpaqueSecurityContext::set_range),
+            ];
+
+            for (new_value, set_value_proc) in list {
+                if let Some(new_value) = new_value {
+                    let c_new_value = os_str_to_c_string(new_value).map_err(|_r| {
+                        let err = io::ErrorKind::InvalidInput.into();
+                        Error::from_io1("Creating security context", path, err)
+                    })?;
+
+                    set_value_proc(&se_context, &c_new_value)
+                        .map_err(|r| Error::from_selinux("Setting security context user", r))?;
                 }
-
-            let mut se_context = selinux::SecurityContext::new(file_context.as_ptr())
-                .map_err(|r| Error::io1("Creating security context", &path, r))?;
-
-            if let Some(user) = user {
-                se_context
-                    .set_user(user)
-                    .map_err(|r| Error::io1("Setting security context user", &path, r))?;
-            }
-
-            if let Some(role) = role {
-                se_context
-                    .set_role(role)
-                    .map_err(|r| Error::io1("Setting security context role", &path, r))?;
-            }
-
-            if let Some(the_type) = the_type {
-                se_context
-                    .set_type(the_type)
-                    .map_err(|r| Error::io1("Setting security context type", &path, r))?;
-            }
-
-            if let Some(range) = range {
-                se_context
-                    .set_range(range)
-                    .map_err(|r| Error::io1("Setting security context range", &path, r))?;
             }
 
             let context_string = se_context
-                .str_bytes()
+                .to_c_string()
-                .map_err(|r| Error::io1("Getting security context", &path, r))?;
+                .map_err(|r| Error::from_selinux("Getting security context", r))?;
 
-            if !file_context.is_empty() && file_context.as_bytes() == context_string {
+            if c_file_context.as_ref().to_bytes() == context_string.as_ref().to_bytes() {
                 Ok(()) // Nothing to change.
             } else {
-                set_file_security_context(
+                SecurityContext::from_c_str(&context_string, false)
-                    &path,
+                    .set_for_path(path, options.affect_symlink_referent, false)
-                    context_string.as_ptr().cast(),
+                    .map_err(|r| Error::from_selinux("Setting security context", r))
-                    options.affect_symlink_referent,
-                )
             }
         }
 
         CommandLineMode::ReferenceBased { .. } | CommandLineMode::ContextBased { .. } => {
-            let path = PathBuf::from(c_str_to_os_string(file));
+            if let Some(c_context) = context.to_c_string()? {
-            let ctx_ptr = context.as_ptr() as *mut c_char;
+                SecurityContext::from_c_str(c_context.as_ref(), false)
-            set_file_security_context(&path, ctx_ptr, options.affect_symlink_referent)
+                    .set_for_path(path, options.affect_symlink_referent, false)
+                    .map_err(|r| Error::from_selinux("Setting security context", r))
+            } else {
+                let err = io::ErrorKind::InvalidInput.into();
+                Err(Error::from_io1("Setting security context", path, err))
+            }
         }
     }
 }
 
 #[cfg(unix)]
-fn c_str_to_os_string(s: &CStr) -> OsString {
+pub(crate) fn os_str_to_c_string(s: &OsStr) -> Result<CString> {
-    use std::os::unix::ffi::OsStringExt;
-
-    OsString::from_vec(s.to_bytes().to_vec())
-}
-
-#[cfg(unix)]
-pub(crate) fn os_str_to_c_string(s: &OsStr) -> io::Result<CString> {
     use std::os::unix::ffi::OsStrExt;
 
-    CString::new(s.as_bytes()).map_err(|_r| io::ErrorKind::InvalidInput.into())
+    CString::new(s.as_bytes())
-}
+        .map_err(|_r| Error::from_io("CString::new()", io::ErrorKind::InvalidInput.into()))
-
-/// SAFETY:
-/// - If `p` is not null, then it is assumed to be a valid null-terminated C string.
-/// - The returned `CStr` must not live more than the data pointed-to by `p`.
-fn ptr_to_c_str<'s>(p: *const c_char) -> io::Result<&'s CStr> {
-    ptr::NonNull::new(p as *mut c_char)
-        .map(|p| unsafe { CStr::from_ptr(p.as_ptr()) })
-        .ok_or_else(|| io::ErrorKind::InvalidInput.into())
 }
 
 /// Call `lstat()` to get the device and inode numbers for `/`.
@@ -776,7 +680,7 @@ fn get_root_dev_ino() -> Result<(libc::ino_t, libc::dev_t)> {
 
     fs::symlink_metadata("/")
         .map(|md| (md.ino(), md.dev()))
-        .map_err(|r| Error::io1("std::fs::symlink_metadata", "/", r))
+        .map_err(|r| Error::from_io1("std::fs::symlink_metadata", "/", r))
 }
 
 fn root_dev_ino_check(
@@ -786,8 +690,8 @@ fn root_dev_ino_check(
     root_dev_ino.map_or(false, |root_dev_ino| root_dev_ino == dir_dev_ino)
 }
 
-fn root_dev_ino_warn(dir_name: &CStr) {
+fn root_dev_ino_warn(dir_name: &Path) {
-    if dir_name.to_bytes() == b"/" {
+    if dir_name.as_os_str() == "/" {
         show_warning!(
             "It is dangerous to operate recursively on '/'. \
              Use --{} to override this failsafe.",
@@ -810,370 +714,37 @@ fn root_dev_ino_warn(dir_name: &CStr) {
 // However, when invoked with "-P -R", it deserves a warning.
 // The fts_options parameter records the options that control this aspect of fts's behavior,
 // so test that.
-fn cycle_warning_required(fts_options: c_int, entry: &fts_sys::FTSENT) -> bool {
+fn cycle_warning_required(fts_options: c_int, entry: &fts::EntryRef) -> bool {
     // When dereferencing no symlinks, or when dereferencing only those listed on the command line
     // and we're not processing a command-line argument, then a cycle is a serious problem.
     ((fts_options & fts_sys::FTS_PHYSICAL) != 0)
-        && (((fts_options & fts_sys::FTS_COMFOLLOW) == 0) || entry.fts_level != 0)
+        && (((fts_options & fts_sys::FTS_COMFOLLOW) == 0) || entry.level() != 0)
 }
 
-fn emit_cycle_warning(file_name: &CStr) {
+fn emit_cycle_warning(file_name: &Path) {
     show_warning!(
         "Circular directory structure.\n\
 This almost certainly means that you have a corrupted file system.\n\
 NOTIFY YOUR SYSTEM MANAGER.\n\
 The following directory is part of the cycle '{}'.",
-        file_name.to_string_lossy()
+        file_name.display()
     )
 }
 
 #[derive(Debug)]
-enum SELinuxSecurityContext {
+enum SELinuxSecurityContext<'t> {
-    File(selinux::FileContext),
+    File(SecurityContext<'t>),
-    String(CString),
+    String(Option<CString>),
 }
 
-impl Default for SELinuxSecurityContext {
+impl<'t> SELinuxSecurityContext<'t> {
-    fn default() -> Self {
+    fn to_c_string(&self) -> Result<Option<Cow<CStr>>> {
-        Self::String(CString::default())
-    }
-}
-
-impl SELinuxSecurityContext {
-    #[cfg(unix)]
-    fn as_ptr(&self) -> *const c_char {
         match self {
-            SELinuxSecurityContext::File(context) => context.as_ptr(),
+            Self::File(context) => context
-            SELinuxSecurityContext::String(context) => context.to_bytes_with_nul().as_ptr().cast(),
+                .to_c_string()
-        }
+                .map_err(|r| Error::from_selinux("SELinuxSecurityContext::to_c_string()", r)),
-    }
+
-}
+            Self::String(context) => Ok(context.as_deref().map(Cow::Borrowed)),
-
-mod fts {
-    use std::ffi::{CStr, CString, OsStr};
-    use std::os::raw::c_int;
-    use std::{io, iter, ptr};
-
-    use super::os_str_to_c_string;
-
-    pub(crate) type FTSOpenCallBack = unsafe extern "C" fn(
-        arg1: *mut *const fts_sys::FTSENT,
-        arg2: *mut *const fts_sys::FTSENT,
-    ) -> c_int;
-
-    #[derive(Debug)]
-    pub(crate) struct FTS {
-        fts: ptr::NonNull<fts_sys::FTS>,
-        entry: *mut fts_sys::FTSENT,
-    }
-
-    impl FTS {
-        pub(crate) fn new<I>(
-            paths: I,
-            options: c_int,
-            compar: Option<FTSOpenCallBack>,
-        ) -> io::Result<Self>
-        where
-            I: IntoIterator,
-            I::Item: AsRef<OsStr>,
-        {
-            let files_paths = paths
-                .into_iter()
-                .map(|s| os_str_to_c_string(s.as_ref()))
-                .collect::<io::Result<Vec<_>>>()?;
-
-            if files_paths.is_empty() {
-                return Err(io::ErrorKind::InvalidInput.into());
-            }
-
-            let path_argv = files_paths
-                .iter()
-                .map(CString::as_ref)
-                .map(CStr::as_ptr)
-                .chain(iter::once(ptr::null()))
-                .collect::<Vec<_>>();
-
-            // SAFETY: We assume calling fts_open() is safe:
-            // - `path_argv` is an array holding at least one path, and null-terminated.
-            let r = unsafe { fts_sys::fts_open(path_argv.as_ptr().cast(), options, compar) };
-            let fts = ptr::NonNull::new(r).ok_or_else(io::Error::last_os_error)?;
-
-            Ok(Self {
-                fts,
-                entry: ptr::null_mut(),
-            })
-        }
-
-        pub(crate) fn read_next_entry(&mut self) -> io::Result<bool> {
-            // SAFETY: We assume calling fts_read() is safe with a non-null `fts`
-            // pointer assumed to be valid.
-            self.entry = unsafe { fts_sys::fts_read(self.fts.as_ptr()) };
-            if self.entry.is_null() {
-                let r = io::Error::last_os_error();
-                if let Some(0) = r.raw_os_error() {
-                    Ok(false)
-                } else {
-                    Err(r)
-                }
-            } else {
-                Ok(true)
-            }
-        }
-
-        pub(crate) fn last_entry_mut(&mut self) -> Option<&mut fts_sys::FTSENT> {
-            // SAFETY: If `self.entry` is not null, then is is assumed to be valid.
-            unsafe { self.entry.as_mut() }
-        }
-
-        pub(crate) fn set(&mut self, instr: c_int) -> io::Result<()> {
-            let fts = self.fts.as_ptr();
-
-            let entry = self
-                .last_entry_mut()
-                .ok_or_else(|| io::Error::from(io::ErrorKind::UnexpectedEof))?;
-
-            // SAFETY: We assume calling fts_set() is safe with non-null `fts`
-            // and `entry` pointers assumed to be valid.
-            if unsafe { fts_sys::fts_set(fts, entry, instr) } == -1 {
-                Err(io::Error::last_os_error())
-            } else {
-                Ok(())
-            }
-        }
-    }
-
-    impl Drop for FTS {
-        fn drop(&mut self) {
-            // SAFETY: We assume calling fts_close() is safe with a non-null `fts`
-            // pointer assumed to be valid.
-            unsafe { fts_sys::fts_close(self.fts.as_ptr()) };
-        }
-    }
-}
-
-mod selinux {
-    use std::ffi::OsStr;
-    use std::os::raw::c_char;
-    use std::path::Path;
-    use std::{io, ptr, slice};
-
-    use super::os_str_to_c_string;
-
-    #[derive(Debug)]
-    pub(crate) struct SecurityContext(ptr::NonNull<selinux_sys::context_s_t>);
-
-    impl SecurityContext {
-        pub(crate) fn new(context_str: *const c_char) -> io::Result<Self> {
-            if context_str.is_null() {
-                Err(io::ErrorKind::InvalidInput.into())
-            } else {
-                // SAFETY: We assume calling context_new() is safe with
-                // a non-null `context_str` pointer assumed to be valid.
-                let p = unsafe { selinux_sys::context_new(context_str) };
-                ptr::NonNull::new(p)
-                    .ok_or_else(io::Error::last_os_error)
-                    .map(Self)
-            }
-        }
-
-        pub(crate) fn is_selinux_enabled() -> bool {
-            // SAFETY: We assume calling is_selinux_enabled() is always safe.
-            unsafe { selinux_sys::is_selinux_enabled() != 0 }
-        }
-
-        pub(crate) fn security_check_context(context: &OsStr) -> io::Result<Option<bool>> {
-            let c_context = os_str_to_c_string(context)?;
-
-            // SAFETY: We assume calling security_check_context() is safe with
-            // a non-null `context` pointer assumed to be valid.
-            if unsafe { selinux_sys::security_check_context(c_context.as_ptr()) } == 0 {
-                Ok(Some(true))
-            } else if Self::is_selinux_enabled() {
-                Ok(Some(false))
-            } else {
-                Ok(None)
-            }
-        }
-
-        pub(crate) fn str_bytes(&self) -> io::Result<&[u8]> {
-            // SAFETY: We assume calling context_str() is safe with
-            // a non-null `context` pointer assumed to be valid.
-            let p = unsafe { selinux_sys::context_str(self.0.as_ptr()) };
-            if p.is_null() {
-                Err(io::ErrorKind::InvalidInput.into())
-            } else {
-                let len = unsafe { libc::strlen(p.cast()) }.saturating_add(1);
-                Ok(unsafe { slice::from_raw_parts(p.cast(), len) })
-            }
-        }
-
-        pub(crate) fn set_user(&mut self, user: &OsStr) -> io::Result<()> {
-            let c_user = os_str_to_c_string(user)?;
-
-            // SAFETY: We assume calling context_user_set() is safe with non-null
-            // `context` and `user` pointers assumed to be valid.
-            if unsafe { selinux_sys::context_user_set(self.0.as_ptr(), c_user.as_ptr()) } == 0 {
-                Ok(())
-            } else {
-                Err(io::Error::last_os_error())
-            }
-        }
-
-        pub(crate) fn set_role(&mut self, role: &OsStr) -> io::Result<()> {
-            let c_role = os_str_to_c_string(role)?;
-
-            // SAFETY: We assume calling context_role_set() is safe with non-null
-            // `context` and `role` pointers assumed to be valid.
-            if unsafe { selinux_sys::context_role_set(self.0.as_ptr(), c_role.as_ptr()) } == 0 {
-                Ok(())
-            } else {
-                Err(io::Error::last_os_error())
-            }
-        }
-
-        pub(crate) fn set_type(&mut self, the_type: &OsStr) -> io::Result<()> {
-            let c_type = os_str_to_c_string(the_type)?;
-
-            // SAFETY: We assume calling context_type_set() is safe with non-null
-            // `context` and `the_type` pointers assumed to be valid.
-            if unsafe { selinux_sys::context_type_set(self.0.as_ptr(), c_type.as_ptr()) } == 0 {
-                Ok(())
-            } else {
-                Err(io::Error::last_os_error())
-            }
-        }
-
-        pub(crate) fn set_range(&mut self, range: &OsStr) -> io::Result<()> {
-            let c_range = os_str_to_c_string(range)?;
-
-            // SAFETY: We assume calling context_range_set() is safe with non-null
-            // `context` and `range` pointers assumed to be valid.
-            if unsafe { selinux_sys::context_range_set(self.0.as_ptr(), c_range.as_ptr()) } == 0 {
-                Ok(())
-            } else {
-                Err(io::Error::last_os_error())
-            }
-        }
-    }
-
-    impl Drop for SecurityContext {
-        fn drop(&mut self) {
-            // SAFETY: We assume calling context_free() is safe with
-            // a non-null `context` pointer assumed to be valid.
-            unsafe { selinux_sys::context_free(self.0.as_ptr()) }
-        }
-    }
-
-    #[derive(Debug)]
-    pub(crate) struct FileContext {
-        pub context: *mut c_char,
-        pub len: usize,
-        pub allocated: bool,
-    }
-
-    impl FileContext {
-        pub(crate) fn new(path: &Path, follow_symbolic_links: bool) -> io::Result<Self> {
-            let c_path = os_str_to_c_string(path.as_os_str())?;
-            let mut context: *mut c_char = ptr::null_mut();
-
-            // SAFETY: We assume calling getfilecon()/lgetfilecon() is safe with
-            // non-null `path` and `context` pointers assumed to be valid.
-            let len = if follow_symbolic_links {
-                unsafe { selinux_sys::getfilecon(c_path.as_ptr(), &mut context) }
-            } else {
-                unsafe { selinux_sys::lgetfilecon(c_path.as_ptr(), &mut context) }
-            };
-
-            if len == -1 {
-                let err = io::Error::last_os_error();
-                if let Some(libc::ENODATA) = err.raw_os_error() {
-                    Ok(Self::default())
-                } else {
-                    Err(err)
-                }
-            } else if context.is_null() {
-                Ok(Self::default())
-            } else {
-                Ok(Self {
-                    context,
-                    len: len as usize,
-                    allocated: true,
-                })
-            }
-        }
-
-        pub(crate) fn from_ptr(context: *mut c_char) -> Self {
-            if context.is_null() {
-                Self::default()
-            } else {
-                // SAFETY: We assume calling strlen() is safe with a non-null
-                // `context` pointer assumed to be valid.
-                let len = unsafe { libc::strlen(context) };
-                Self {
-                    context,
-                    len,
-                    allocated: false,
-                }
-            }
-        }
-
-        pub(crate) fn set_for_file(
-            &mut self,
-            path: &Path,
-            follow_symbolic_links: bool,
-        ) -> io::Result<()> {
-            let c_path = os_str_to_c_string(path.as_os_str())?;
-
-            // SAFETY: We assume calling setfilecon()/lsetfilecon() is safe with
-            // non-null `path` and `context` pointers assumed to be valid.
-            let r = if follow_symbolic_links {
-                unsafe { selinux_sys::setfilecon(c_path.as_ptr(), self.context) }
-            } else {
-                unsafe { selinux_sys::lsetfilecon(c_path.as_ptr(), self.context) }
-            };
-
-            if r == -1 {
-                Err(io::Error::last_os_error())
-            } else {
-                Ok(())
-            }
-        }
-
-        pub(crate) fn as_ptr(&self) -> *const c_char {
-            self.context
-        }
-
-        pub(crate) fn is_empty(&self) -> bool {
-            self.context.is_null() || self.len == 0
-        }
-
-        pub(crate) fn as_bytes(&self) -> &[u8] {
-            if self.context.is_null() {
-                &[]
-            } else {
-                // SAFETY: `self.0.context` is a non-null pointer that is assumed to be valid.
-                unsafe { slice::from_raw_parts(self.context.cast(), self.len) }
-            }
-        }
-    }
-
-    impl Default for FileContext {
-        fn default() -> Self {
-            Self {
-                context: ptr::null_mut(),
-                len: 0,
-                allocated: false,
-            }
-        }
-    }
-
-    impl Drop for FileContext {
-        fn drop(&mut self) {
-            if self.allocated && !self.context.is_null() {
-                // SAFETY: We assume calling freecon() is safe with a non-null
-                // `context` pointer assumed to be valid.
-                unsafe { selinux_sys::freecon(self.context) }
-            }
         }
     }
 }

src/uu/chcon/src/errors.rs

@@ -0,0 +1,71 @@
+use std::ffi::OsString;
+use std::fmt::Write;
+use std::io;
+
+pub(crate) type Result<T> = std::result::Result<T, Error>;
+
+#[derive(thiserror::Error, Debug)]
+pub(crate) enum Error {
+    #[error("No context is specified")]
+    MissingContext,
+
+    #[error("No files are specified")]
+    MissingFiles,
+
+    #[error("{0}")]
+    ArgumentsMismatch(String),
+
+    #[error(transparent)]
+    CommandLine(#[from] clap::Error),
+
+    #[error("{operation} failed")]
+    SELinux {
+        operation: &'static str,
+        source: selinux::errors::Error,
+    },
+
+    #[error("{operation} failed")]
+    Io {
+        operation: &'static str,
+        source: io::Error,
+    },
+
+    #[error("{operation} failed on '{}'", .operand1.to_string_lossy())]
+    Io1 {
+        operation: &'static str,
+        operand1: OsString,
+        source: io::Error,
+    },
+}
+
+impl Error {
+    pub(crate) fn from_io(operation: &'static str, source: io::Error) -> Self {
+        Self::Io { operation, source }
+    }
+
+    pub(crate) fn from_io1(
+        operation: &'static str,
+        operand1: impl Into<OsString>,
+        source: io::Error,
+    ) -> Self {
+        Self::Io1 {
+            operation,
+            operand1: operand1.into(),
+            source,
+        }
+    }
+
+    pub(crate) fn from_selinux(operation: &'static str, source: selinux::errors::Error) -> Self {
+        Self::SELinux { operation, source }
+    }
+}
+
+pub(crate) fn report_full_error(mut err: &dyn std::error::Error) -> String {
+    let mut desc = String::with_capacity(256);
+    write!(&mut desc, "{}", err).unwrap();
+    while let Some(source) = err.source() {
+        err = source;
+        write!(&mut desc, ". {}", err).unwrap();
+    }
+    desc
+}

src/uu/chcon/src/fts.rs

@@ -0,0 +1,193 @@
+use std::ffi::{CStr, CString, OsStr};
+use std::marker::PhantomData;
+use std::os::raw::{c_int, c_long, c_short};
+use std::path::Path;
+use std::ptr::NonNull;
+use std::{io, iter, ptr, slice};
+
+use crate::errors::{Error, Result};
+use crate::os_str_to_c_string;
+
+#[derive(Debug)]
+pub(crate) struct FTS {
+    fts: ptr::NonNull<fts_sys::FTS>,
+
+    entry: Option<ptr::NonNull<fts_sys::FTSENT>>,
+    _phantom_data: PhantomData<fts_sys::FTSENT>,
+}
+
+impl FTS {
+    pub(crate) fn new<I>(paths: I, options: c_int) -> Result<Self>
+    where
+        I: IntoIterator,
+        I::Item: AsRef<OsStr>,
+    {
+        let files_paths: Vec<CString> = paths
+            .into_iter()
+            .map(|s| os_str_to_c_string(s.as_ref()))
+            .collect::<Result<_>>()?;
+
+        if files_paths.is_empty() {
+            return Err(Error::from_io(
+                "FTS::new()",
+                io::ErrorKind::InvalidInput.into(),
+            ));
+        }
+
+        let path_argv: Vec<_> = files_paths
+            .iter()
+            .map(CString::as_ref)
+            .map(CStr::as_ptr)
+            .chain(iter::once(ptr::null()))
+            .collect();
+
+        // SAFETY: We assume calling fts_open() is safe:
+        // - `path_argv` is an array holding at least one path, and null-terminated.
+        // - `compar` is None.
+        let fts = unsafe { fts_sys::fts_open(path_argv.as_ptr().cast(), options, None) };
+
+        let fts = ptr::NonNull::new(fts)
+            .ok_or_else(|| Error::from_io("fts_open()", io::Error::last_os_error()))?;
+
+        Ok(Self {
+            fts,
+            entry: None,
+            _phantom_data: PhantomData,
+        })
+    }
+
+    pub(crate) fn last_entry_ref(&mut self) -> Option<EntryRef> {
+        self.entry.map(move |entry| EntryRef::new(self, entry))
+    }
+
+    pub(crate) fn read_next_entry(&mut self) -> Result<bool> {
+        // SAFETY: We assume calling fts_read() is safe with a non-null `fts`
+        // pointer assumed to be valid.
+        let new_entry = unsafe { fts_sys::fts_read(self.fts.as_ptr()) };
+
+        self.entry = NonNull::new(new_entry);
+        if self.entry.is_none() {
+            let r = io::Error::last_os_error();
+            if let Some(0) = r.raw_os_error() {
+                Ok(false)
+            } else {
+                Err(Error::from_io("fts_read()", r))
+            }
+        } else {
+            Ok(true)
+        }
+    }
+
+    pub(crate) fn set(&mut self, instr: c_int) -> Result<()> {
+        let fts = self.fts.as_ptr();
+        let entry = self
+            .entry
+            .ok_or_else(|| Error::from_io("FTS::set()", io::ErrorKind::UnexpectedEof.into()))?;
+
+        // SAFETY: We assume calling fts_set() is safe with non-null `fts`
+        // and `entry` pointers assumed to be valid.
+        if unsafe { fts_sys::fts_set(fts, entry.as_ptr(), instr) } == -1 {
+            Err(Error::from_io("fts_set()", io::Error::last_os_error()))
+        } else {
+            Ok(())
+        }
+    }
+}
+
+impl Drop for FTS {
+    fn drop(&mut self) {
+        // SAFETY: We assume calling fts_close() is safe with a non-null `fts`
+        // pointer assumed to be valid.
+        unsafe { fts_sys::fts_close(self.fts.as_ptr()) };
+    }
+}
+
+#[derive(Debug)]
+pub(crate) struct EntryRef<'fts> {
+    pub(crate) pointer: ptr::NonNull<fts_sys::FTSENT>,
+
+    _fts: PhantomData<&'fts FTS>,
+    _phantom_data: PhantomData<fts_sys::FTSENT>,
+}
+
+impl<'fts> EntryRef<'fts> {
+    fn new(_fts: &'fts FTS, entry: ptr::NonNull<fts_sys::FTSENT>) -> Self {
+        Self {
+            pointer: entry,
+            _fts: PhantomData,
+            _phantom_data: PhantomData,
+        }
+    }
+
+    fn as_ref(&self) -> &fts_sys::FTSENT {
+        // SAFETY: `self.pointer` is a non-null pointer that is assumed to be valid.
+        unsafe { self.pointer.as_ref() }
+    }
+
+    fn as_mut(&mut self) -> &mut fts_sys::FTSENT {
+        // SAFETY: `self.pointer` is a non-null pointer that is assumed to be valid.
+        unsafe { self.pointer.as_mut() }
+    }
+
+    pub(crate) fn flags(&self) -> c_int {
+        c_int::from(self.as_ref().fts_info)
+    }
+
+    pub(crate) fn errno(&self) -> c_int {
+        self.as_ref().fts_errno
+    }
+
+    pub(crate) fn level(&self) -> c_short {
+        self.as_ref().fts_level
+    }
+
+    pub(crate) fn number(&self) -> c_long {
+        self.as_ref().fts_number
+    }
+
+    pub(crate) fn set_number(&mut self, new_number: c_long) {
+        self.as_mut().fts_number = new_number;
+    }
+
+    pub(crate) fn path(&self) -> Option<&Path> {
+        let entry = self.as_ref();
+        if entry.fts_pathlen == 0 {
+            return None;
+        }
+
+        NonNull::new(entry.fts_path)
+            .map(|path_ptr| {
+                let path_size = usize::from(entry.fts_pathlen).saturating_add(1);
+
+                // SAFETY: `entry.fts_path` is a non-null pointer that is assumed to be valid.
+                unsafe { slice::from_raw_parts(path_ptr.as_ptr().cast(), path_size) }
+            })
+            .and_then(|bytes| CStr::from_bytes_with_nul(bytes).ok())
+            .map(c_str_to_os_str)
+            .map(Path::new)
+    }
+
+    pub(crate) fn access_path(&self) -> Option<&Path> {
+        ptr::NonNull::new(self.as_ref().fts_accpath)
+            .map(|path_ptr| {
+                // SAFETY: `entry.fts_accpath` is a non-null pointer that is assumed to be valid.
+                unsafe { CStr::from_ptr(path_ptr.as_ptr()) }
+            })
+            .map(c_str_to_os_str)
+            .map(Path::new)
+    }
+
+    pub(crate) fn stat(&self) -> Option<&libc::stat> {
+        ptr::NonNull::new(self.as_ref().fts_statp).map(|stat_ptr| {
+            // SAFETY: `entry.fts_statp` is a non-null pointer that is assumed to be valid.
+            unsafe { stat_ptr.as_ref() }
+        })
+    }
+}
+
+#[cfg(unix)]
+fn c_str_to_os_str(s: &CStr) -> &OsStr {
+    use std::os::unix::ffi::OsStrExt;
+
+    OsStr::from_bytes(s.to_bytes())
+}

src/uu/chgrp/src/chgrp.rs

@@ -161,12 +161,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         Verbosity::Normal
     };
 
-    let dest_gid: u32;
+    let dest_gid = if let Some(file) = matches.value_of(options::REFERENCE) {
-    if let Some(file) = matches.value_of(options::REFERENCE) {
         match fs::metadata(&file) {
-            Ok(meta) => {
+            Ok(meta) => Some(meta.gid()),
-                dest_gid = meta.gid();
-            }
             Err(e) => {
                 show_error!("failed to get attributes of '{}': {}", file, e);
                 return 1;
@@ -174,16 +171,18 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         }
     } else {
         let group = matches.value_of(options::ARG_GROUP).unwrap_or_default();
+        if group.is_empty() {
+            None
+        } else {
             match entries::grp2gid(group) {
-            Ok(g) => {
+                Ok(g) => Some(g),
-                dest_gid = g;
-            }
                 _ => {
                     show_error!("invalid group: {}", group);
                     return 1;
                 }
             }
         }
+    };
 
     let executor = Chgrper {
         bit_flag,
@@ -278,7 +277,7 @@ pub fn uu_app() -> App<'static, 'static> {
 }
 
 struct Chgrper {
-    dest_gid: gid_t,
+    dest_gid: Option<gid_t>,
     bit_flag: u8,
     verbosity: Verbosity,
     files: Vec<String>,

src/uu/chown/src/chown.rs

@@ -14,7 +14,7 @@ use uucore::fs::resolve_relative_path;
 use uucore::libc::{gid_t, uid_t};
 use uucore::perms::{wrap_chown, Verbosity};
 
-use uucore::error::{FromIo, UError, UResult, USimpleError};
+use uucore::error::{FromIo, UResult, USimpleError};
 
 use clap::{crate_version, App, Arg};
 
@@ -107,10 +107,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     if recursive {
         if bit_flag == FTS_PHYSICAL {
             if derefer == 1 {
-                return Err(USimpleError::new(
+                return Err(USimpleError::new(1, "-R --dereference requires -H or -L"));
-                    1,
-                    "-R --dereference requires -H or -L".to_string(),
-                ));
             }
             derefer = 0;
         }
@@ -324,7 +321,7 @@ impl Chowner {
             ret |= self.traverse(f);
         }
         if ret != 0 {
-            return Err(UError::from(ret));
+            return Err(ret.into());
         }
         Ok(())
     }

src/uu/csplit/src/split_name.rs

@@ -47,7 +47,7 @@ impl SplitName {
             }),
             Some(custom) => {
                 let spec =
-                    Regex::new(r"(?P<ALL>%(?P<FLAG>[0#-])(?P<WIDTH>\d+)?(?P<TYPE>[diuoxX]))")
+                    Regex::new(r"(?P<ALL>%((?P<FLAG>[0#-])(?P<WIDTH>\d+)?)?(?P<TYPE>[diuoxX]))")
                         .unwrap();
                 let mut captures_iter = spec.captures_iter(&custom);
                 let custom_fn: Box<dyn Fn(usize) -> String> = match captures_iter.next() {
@@ -60,6 +60,21 @@ impl SplitName {
                             Some(m) => m.as_str().parse::<usize>().unwrap(),
                         };
                         match (captures.name("FLAG"), captures.name("TYPE")) {
+                            (None, Some(ref t)) => match t.as_str() {
+                                "d" | "i" | "u" => Box::new(move |n: usize| -> String {
+                                    format!("{}{}{}{}", prefix, before, n, after)
+                                }),
+                                "o" => Box::new(move |n: usize| -> String {
+                                    format!("{}{}{:o}{}", prefix, before, n, after)
+                                }),
+                                "x" => Box::new(move |n: usize| -> String {
+                                    format!("{}{}{:x}{}", prefix, before, n, after)
+                                }),
+                                "X" => Box::new(move |n: usize| -> String {
+                                    format!("{}{}{:X}{}", prefix, before, n, after)
+                                }),
+                                _ => return Err(CsplitError::SuffixFormatIncorrect),
+                            },
                             (Some(ref f), Some(ref t)) => {
                                 match (f.as_str(), t.as_str()) {
                                     /*
@@ -276,6 +291,12 @@ mod tests {
         assert_eq!(split_name.get(2), "xx00002");
     }
 
+    #[test]
+    fn no_padding_decimal() {
+        let split_name = SplitName::new(None, Some(String::from("cst-%d-")), None).unwrap();
+        assert_eq!(split_name.get(2), "xxcst-2-");
+    }
+
     #[test]
     fn zero_padding_decimal1() {
         let split_name = SplitName::new(None, Some(String::from("cst-%03d-")), None).unwrap();

src/uu/df/src/df.rs

@@ -8,7 +8,7 @@
 
 #[macro_use]
 extern crate uucore;
-use uucore::error::UCustomError;
+use uucore::error::UError;
 use uucore::error::UResult;
 #[cfg(unix)]
 use uucore::fsext::statfs_fn;
@@ -274,7 +274,7 @@ impl Display for DfError {
 
 impl Error for DfError {}
 
-impl UCustomError for DfError {
+impl UError for DfError {
     fn code(&self) -> i32 {
         match self {
             DfError::InvalidBaseValue(_) => 1,

src/uu/dirname/src/dirname.rs

@@ -79,7 +79,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
             print!("{}", separator);
         }
     } else {
-        return Err(UUsageError::new(1, "missing operand".to_string()));
+        return Err(UUsageError::new(1, "missing operand"));
     }
 
     Ok(())

src/uu/du/src/du.rs

@@ -32,7 +32,7 @@ use std::path::PathBuf;
 use std::str::FromStr;
 use std::time::{Duration, UNIX_EPOCH};
 use std::{error::Error, fmt::Display};
-use uucore::error::{UCustomError, UResult};
+use uucore::error::{UError, UResult};
 use uucore::parse_size::{parse_size, ParseSizeError};
 use uucore::InvalidEncodingHandling;
 #[cfg(windows)]
@@ -438,7 +438,7 @@ Try '{} --help' for more information.",
 
 impl Error for DuError {}
 
-impl UCustomError for DuError {
+impl UError for DuError {
     fn code(&self) -> i32 {
         match self {
             Self::InvalidMaxDepthArg(_) => 1,

src/uu/factor/src/cli.rs

@@ -10,6 +10,7 @@
 extern crate uucore;
 
 use std::error::Error;
+use std::fmt::Write as FmtWrite;
 use std::io::{self, stdin, stdout, BufRead, Write};
 
 mod factor;
@@ -28,21 +29,29 @@ mod options {
     pub static NUMBER: &str = "NUMBER";
 }
 
-fn print_factors_str(num_str: &str, w: &mut impl io::Write) -> Result<(), Box<dyn Error>> {
+fn print_factors_str(
-    num_str
+    num_str: &str,
-        .parse::<u64>()
+    w: &mut io::BufWriter<impl io::Write>,
-        .map_err(|e| e.into())
+    factors_buffer: &mut String,
-        .and_then(|x| writeln!(w, "{}:{}", x, factor(x)).map_err(|e| e.into()))
+) -> Result<(), Box<dyn Error>> {
+    num_str.parse::<u64>().map_err(|e| e.into()).and_then(|x| {
+        factors_buffer.clear();
+        writeln!(factors_buffer, "{}:{}", x, factor(x))?;
+        w.write_all(factors_buffer.as_bytes())?;
+        Ok(())
+    })
 }
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
     let matches = uu_app().get_matches_from(args);
     let stdout = stdout();
-    let mut w = io::BufWriter::new(stdout.lock());
+    // We use a smaller buffer here to pass a gnu test. 4KiB appears to be the default pipe size for bash.
+    let mut w = io::BufWriter::with_capacity(4 * 1024, stdout.lock());
+    let mut factors_buffer = String::new();
 
     if let Some(values) = matches.values_of(options::NUMBER) {
         for number in values {
-            if let Err(e) = print_factors_str(number, &mut w) {
+            if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) {
                 show_warning!("{}: {}", number, e);
             }
         }
@@ -51,7 +60,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
         for line in stdin.lock().lines() {
             for number in line.unwrap().split_whitespace() {
-                if let Err(e) = print_factors_str(number, &mut w) {
+                if let Err(e) = print_factors_str(number, &mut w, &mut factors_buffer) {
                     show_warning!("{}: {}", number, e);
                 }
             }

src/uu/false/src/false.rs

@@ -9,13 +9,12 @@
 extern crate uucore;
 
 use clap::App;
-use uucore::error::{UError, UResult};
+use uucore::{error::UResult, executable};
-use uucore::executable;
 
 #[uucore_procs::gen_uumain]
 pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     uu_app().get_matches_from(args);
-    Err(UError::from(1))
+    Err(1.into())
 }
 
 pub fn uu_app() -> App<'static, 'static> {

src/uu/id/Cargo.toml

@@ -18,7 +18,7 @@ path = "src/id.rs"
 clap = { version = "2.33", features = ["wrap_help"] }
 uucore = { version=">=0.0.9", package="uucore", path="../../uucore", features=["entries", "process"] }
 uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" }
-selinux = { version="0.1.3", optional = true }
+selinux = { version="0.2.1", optional = true }
 
 [[bin]]
 name = "id"

src/uu/id/src/id.rs

@@ -40,10 +40,10 @@
 extern crate uucore;
 
 use clap::{crate_version, App, Arg};
-#[cfg(all(target_os = "linux", feature = "selinux"))]
-use selinux;
 use std::ffi::CStr;
 use uucore::entries::{self, Group, Locate, Passwd};
+use uucore::error::UResult;
+use uucore::error::{set_exit_code, USimpleError};
 pub use uucore::libc;
 use uucore::libc::{getlogin, uid_t};
 use uucore::process::{getegid, geteuid, getgid, getuid};
@@ -123,10 +123,10 @@ struct State {
     // 1000 10 968 975
     // +++ exited with 0 +++
     user_specified: bool,
-    exit_code: i32,
 }
 
-pub fn uumain(args: impl uucore::Args) -> i32 {
+#[uucore_procs::gen_uumain]
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let usage = get_usage();
     let after_help = get_description();
 
@@ -161,7 +161,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         },
         user_specified: !users.is_empty(),
         ids: None,
-        exit_code: 0,
     };
 
     let default_format = {
@@ -170,14 +169,23 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     };
 
     if (state.nflag || state.rflag) && default_format && !state.cflag {
-        crash!(1, "cannot print only names or real IDs in default format");
+        return Err(USimpleError::new(
+            1,
+            "cannot print only names or real IDs in default format",
+        ));
     }
     if state.zflag && default_format && !state.cflag {
         // NOTE: GNU test suite "id/zero.sh" needs this stderr output:
-        crash!(1, "option --zero not permitted in default format");
+        return Err(USimpleError::new(
+            1,
+            "option --zero not permitted in default format",
+        ));
     }
     if state.user_specified && state.cflag {
-        crash!(1, "cannot print security context when user specified");
+        return Err(USimpleError::new(
+            1,
+            "cannot print security context when user specified",
+        ));
     }
 
     let delimiter = {
@@ -204,11 +212,14 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 print!("{}{}", String::from_utf8_lossy(bytes), line_ending);
             } else {
                 // print error because `cflag` was explicitly requested
-                crash!(1, "can't get process context");
+                return Err(USimpleError::new(1, "can't get process context"));
             }
-            return state.exit_code;
+            return Ok(());
         } else {
-            crash!(1, "--context (-Z) works only on an SELinux-enabled kernel");
+            return Err(USimpleError::new(
+                1,
+                "--context (-Z) works only on an SELinux-enabled kernel",
+            ));
         }
     }
 
@@ -220,7 +231,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 Ok(p) => Some(p),
                 Err(_) => {
                     show_error!("'{}': no such user", users[i]);
-                    state.exit_code = 1;
+                    set_exit_code(1);
                     if i + 1 >= users.len() {
                         break;
                     } else {
@@ -234,17 +245,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         if matches.is_present(options::OPT_PASSWORD) {
             // BSD's `id` ignores all but the first specified user
             pline(possible_pw.map(|v| v.uid()));
-            return state.exit_code;
+            return Ok(());
         };
         if matches.is_present(options::OPT_HUMAN_READABLE) {
             // BSD's `id` ignores all but the first specified user
             pretty(possible_pw);
-            return state.exit_code;
+            return Ok(());
         }
         if matches.is_present(options::OPT_AUDIT) {
             // BSD's `id` ignores specified users
             auditid();
-            return state.exit_code;
+            return Ok(());
         }
 
         let (uid, gid) = possible_pw.map(|p| (p.uid(), p.gid())).unwrap_or((
@@ -264,7 +275,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 if state.nflag {
                     entries::gid2grp(gid).unwrap_or_else(|_| {
                         show_error!("cannot find name for group ID {}", gid);
-                        state.exit_code = 1;
+                        set_exit_code(1);
                         gid.to_string()
                     })
                 } else {
@@ -279,7 +290,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 if state.nflag {
                     entries::uid2usr(uid).unwrap_or_else(|_| {
                         show_error!("cannot find name for user ID {}", uid);
-                        state.exit_code = 1;
+                        set_exit_code(1);
                         uid.to_string()
                     })
                 } else {
@@ -304,7 +315,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                         if state.nflag {
                             entries::gid2grp(id).unwrap_or_else(|_| {
                                 show_error!("cannot find name for group ID {}", id);
-                                state.exit_code = 1;
+                                set_exit_code(1);
                                 id.to_string()
                             })
                         } else {
@@ -332,7 +343,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         }
     }
 
-    state.exit_code
+    Ok(())
 }
 
 pub fn uu_app() -> App<'static, 'static> {
@@ -560,7 +571,7 @@ fn id_print(state: &mut State, groups: Vec<u32>) {
         uid,
         entries::uid2usr(uid).unwrap_or_else(|_| {
             show_error!("cannot find name for user ID {}", uid);
-            state.exit_code = 1;
+            set_exit_code(1);
             uid.to_string()
         })
     );
@@ -569,7 +580,7 @@ fn id_print(state: &mut State, groups: Vec<u32>) {
         gid,
         entries::gid2grp(gid).unwrap_or_else(|_| {
             show_error!("cannot find name for group ID {}", gid);
-            state.exit_code = 1;
+            set_exit_code(1);
             gid.to_string()
         })
     );
@@ -579,7 +590,7 @@ fn id_print(state: &mut State, groups: Vec<u32>) {
             euid,
             entries::uid2usr(euid).unwrap_or_else(|_| {
                 show_error!("cannot find name for user ID {}", euid);
-                state.exit_code = 1;
+                set_exit_code(1);
                 euid.to_string()
             })
         );
@@ -590,7 +601,7 @@ fn id_print(state: &mut State, groups: Vec<u32>) {
             euid,
             entries::gid2grp(egid).unwrap_or_else(|_| {
                 show_error!("cannot find name for group ID {}", egid);
-                state.exit_code = 1;
+                set_exit_code(1);
                 egid.to_string()
             })
         );
@@ -604,7 +615,7 @@ fn id_print(state: &mut State, groups: Vec<u32>) {
                 gr,
                 entries::gid2grp(gr).unwrap_or_else(|_| {
                     show_error!("cannot find name for group ID {}", gr);
-                    state.exit_code = 1;
+                    set_exit_code(1);
                     gr.to_string()
                 })
             ))

src/uu/install/src/install.rs

@@ -5,7 +5,7 @@
 //  * For the full copyright and license information, please view the LICENSE file
 //  * that was distributed with this source code.
 
-// spell-checker:ignore (ToDO) rwxr sourcepath targetpath
+// spell-checker:ignore (ToDO) rwxr sourcepath targetpath Isnt uioerror
 
 mod mode;
 
@@ -17,15 +17,17 @@ use file_diff::diff;
 use filetime::{set_file_times, FileTime};
 use uucore::backup_control::{self, BackupMode};
 use uucore::entries::{grp2gid, usr2uid};
+use uucore::error::{FromIo, UError, UIoError, UResult, USimpleError};
 use uucore::perms::{wrap_chgrp, wrap_chown, Verbosity};
 
 use libc::{getegid, geteuid};
+use std::error::Error;
+use std::fmt::{Debug, Display};
 use std::fs;
 use std::fs::File;
 use std::os::unix::fs::MetadataExt;
 use std::path::{Path, PathBuf};
 use std::process::Command;
-use std::result::Result;
 
 const DEFAULT_MODE: u32 = 0o755;
 const DEFAULT_STRIP_PROGRAM: &str = "strip";
@@ -47,6 +49,87 @@ pub struct Behavior {
     target_dir: Option<String>,
 }
 
+#[derive(Debug)]
+enum InstallError {
+    Unimplemented(String),
+    DirNeedsArg(),
+    CreateDirFailed(PathBuf, std::io::Error),
+    ChmodFailed(PathBuf),
+    InvalidTarget(PathBuf),
+    TargetDirIsntDir(PathBuf),
+    BackupFailed(PathBuf, PathBuf, std::io::Error),
+    InstallFailed(PathBuf, PathBuf, std::io::Error),
+    StripProgramFailed(String),
+    MetadataFailed(std::io::Error),
+    NoSuchUser(String),
+    NoSuchGroup(String),
+    OmittingDirectory(PathBuf),
+}
+
+impl UError for InstallError {
+    fn code(&self) -> i32 {
+        match self {
+            InstallError::Unimplemented(_) => 2,
+            _ => 1,
+        }
+    }
+
+    fn usage(&self) -> bool {
+        false
+    }
+}
+
+impl Error for InstallError {}
+
+impl Display for InstallError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        use InstallError as IE;
+        match self {
+            IE::Unimplemented(opt) => write!(f, "Unimplemented feature: {}", opt),
+            IE::DirNeedsArg() => write!(
+                f,
+                "{} with -d requires at least one argument.",
+                executable!()
+            ),
+            IE::CreateDirFailed(dir, e) => {
+                Display::fmt(&uio_error!(e, "failed to create {}", dir.display()), f)
+            }
+            IE::ChmodFailed(file) => write!(f, "failed to chmod {}", file.display()),
+            IE::InvalidTarget(target) => write!(
+                f,
+                "invalid target {}: No such file or directory",
+                target.display()
+            ),
+            IE::TargetDirIsntDir(target) => {
+                write!(f, "target '{}' is not a directory", target.display())
+            }
+            IE::BackupFailed(from, to, e) => Display::fmt(
+                &uio_error!(
+                    e,
+                    "cannot backup '{}' to '{}'",
+                    from.display(),
+                    to.display()
+                ),
+                f,
+            ),
+            IE::InstallFailed(from, to, e) => Display::fmt(
+                &uio_error!(
+                    e,
+                    "cannot install '{}' to '{}'",
+                    from.display(),
+                    to.display()
+                ),
+                f,
+            ),
+            IE::StripProgramFailed(msg) => write!(f, "strip program failed: {}", msg),
+            IE::MetadataFailed(e) => Display::fmt(&uio_error!(e, ""), f),
+            IE::NoSuchUser(user) => write!(f, "no such user: {}", user),
+            IE::NoSuchGroup(group) => write!(f, "no such group: {}", group),
+            IE::OmittingDirectory(dir) => write!(f, "omitting directory '{}'", dir.display()),
+        }
+    }
+}
+
 #[derive(Clone, Eq, PartialEq)]
 pub enum MainFunction {
     /// Create directories
@@ -97,7 +180,8 @@ fn get_usage() -> String {
 ///
 /// Returns a program return code.
 ///
-pub fn uumain(args: impl uucore::Args) -> i32 {
+#[uucore_procs::gen_uumain]
+pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     let usage = get_usage();
 
     let matches = uu_app().usage(&usage[..]).get_matches_from(args);
@@ -107,17 +191,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         .map(|v| v.map(ToString::to_string).collect())
         .unwrap_or_default();
 
-    if let Err(s) = check_unimplemented(&matches) {
+    check_unimplemented(&matches)?;
-        show_error!("Unimplemented feature: {}", s);
-        return 2;
-    }
 
-    let behavior = match behavior(&matches) {
+    let behavior = behavior(&matches)?;
-        Ok(x) => x,
-        Err(ret) => {
-            return ret;
-        }
-    };
 
     match behavior.main_function {
         MainFunction::Directory => directory(paths, behavior),
@@ -269,13 +345,13 @@ pub fn uu_app() -> App<'static, 'static> {
 /// Error datum is a string of the unimplemented argument.
 ///
 ///
-fn check_unimplemented<'a>(matches: &ArgMatches) -> Result<(), &'a str> {
+fn check_unimplemented(matches: &ArgMatches) -> UResult<()> {
     if matches.is_present(OPT_NO_TARGET_DIRECTORY) {
-        Err("--no-target-directory, -T")
+        Err(InstallError::Unimplemented(String::from("--no-target-directory, -T")).into())
     } else if matches.is_present(OPT_PRESERVE_CONTEXT) {
-        Err("--preserve-context, -P")
+        Err(InstallError::Unimplemented(String::from("--preserve-context, -P")).into())
     } else if matches.is_present(OPT_CONTEXT) {
-        Err("--context, -Z")
+        Err(InstallError::Unimplemented(String::from("--context, -Z")).into())
     } else {
         Ok(())
     }
@@ -289,7 +365,7 @@ fn check_unimplemented<'a>(matches: &ArgMatches) -> Result<(), &'a str> {
 ///
 /// In event of failure, returns an integer intended as a program return code.
 ///
-fn behavior(matches: &ArgMatches) -> Result<Behavior, i32> {
+fn behavior(matches: &ArgMatches) -> UResult<Behavior> {
     let main_function = if matches.is_present(OPT_DIRECTORY) {
         MainFunction::Directory
     } else {
@@ -314,10 +390,7 @@ fn behavior(matches: &ArgMatches) -> Result<Behavior, i32> {
         matches.value_of(OPT_BACKUP),
     );
     let backup_mode = match backup_mode {
-        Err(err) => {
+        Err(err) => return Err(USimpleError::new(1, err)),
-            show_usage_error!("{}", err);
-            return Err(1);
-        }
         Ok(mode) => mode,
     };
 
@@ -349,45 +422,46 @@ fn behavior(matches: &ArgMatches) -> Result<Behavior, i32> {
 /// GNU man pages describe this functionality as creating 'all components of
 /// the specified directories'.
 ///
-/// Returns an integer intended as a program return code.
+/// Returns a Result type with the Err variant containing the error message.
 ///
-fn directory(paths: Vec<String>, b: Behavior) -> i32 {
+fn directory(paths: Vec<String>, b: Behavior) -> UResult<()> {
     if paths.is_empty() {
-        println!("{} with -d requires at least one argument.", executable!());
+        Err(InstallError::DirNeedsArg().into())
-        1
     } else {
-        let mut all_successful = true;
-
         for path in paths.iter().map(Path::new) {
             // if the path already exist, don't try to create it again
             if !path.exists() {
-                // Differently than the primary functionality (MainFunction::Standard), the directory
+                // Differently than the primary functionality
-                // functionality should create all ancestors (or components) of a directory regardless
+                // (MainFunction::Standard), the directory functionality should
-                // of the presence of the "-D" flag.
+                // create all ancestors (or components) of a directory
-                // NOTE: the GNU "install" sets the expected mode only for the target directory. All
+                // regardless of the presence of the "-D" flag.
-                // created ancestor directories will have the default mode. Hence it is safe to use
+                //
-                // fs::create_dir_all and then only modify the target's dir mode.
+                // NOTE: the GNU "install" sets the expected mode only for the
-                if let Err(e) = fs::create_dir_all(path) {
+                // target directory. All created ancestor directories will have
-                    show_error!("{}: {}", path.display(), e);
+                // the default mode. Hence it is safe to use fs::create_dir_all
-                    all_successful = false;
+                // and then only modify the target's dir mode.
+                if let Err(e) =
+                    fs::create_dir_all(path).map_err_context(|| format!("{}", path.display()))
+                {
+                    show!(e);
                     continue;
                 }
 
                 if b.verbose {
-                    show_error!("creating directory '{}'", path.display());
+                    println!("creating directory '{}'", path.display());
                 }
             }
 
             if mode::chmod(path, b.mode()).is_err() {
-                all_successful = false;
+                // Error messages are printed by the mode::chmod function!
+                uucore::error::set_exit_code(1);
                 continue;
             }
         }
-        if all_successful {
+        // If the exit code was set, or show! has been called at least once
-            0
+        // (which sets the exit code as well), function execution will end after
-        } else {
+        // this return.
-            1
+        Ok(())
-        }
     }
 }
 
@@ -401,9 +475,9 @@ fn is_new_file_path(path: &Path) -> bool {
 
 /// Perform an install, given a list of paths and behavior.
 ///
-/// Returns an integer intended as a program return code.
+/// Returns a Result type with the Err variant containing the error message.
 ///
-fn standard(mut paths: Vec<String>, b: Behavior) -> i32 {
+fn standard(mut paths: Vec<String>, b: Behavior) -> UResult<()> {
     let target: PathBuf = b
         .target_dir
         .clone()
@@ -418,25 +492,19 @@ fn standard(mut paths: Vec<String>, b: Behavior) -> i32 {
         if let Some(parent) = target.parent() {
             if !parent.exists() && b.create_leading {
                 if let Err(e) = fs::create_dir_all(parent) {
-                    show_error!("failed to create {}: {}", parent.display(), e);
+                    return Err(InstallError::CreateDirFailed(parent.to_path_buf(), e).into());
-                    return 1;
                 }
 
                 if mode::chmod(parent, b.mode()).is_err() {
-                    show_error!("failed to chmod {}", parent.display());
+                    return Err(InstallError::ChmodFailed(parent.to_path_buf()).into());
-                    return 1;
                 }
             }
         }
 
         if target.is_file() || is_new_file_path(&target) {
-            copy_file_to_file(&sources[0], &target, &b)
+            copy(&sources[0], &target, &b)
         } else {
-            show_error!(
+            Err(InstallError::InvalidTarget(target).into())
-                "invalid target {}: No such file or directory",
-                target.display()
-            );
-            1
         }
     }
 }
@@ -444,34 +512,30 @@ fn standard(mut paths: Vec<String>, b: Behavior) -> i32 {
 /// Copy some files into a directory.
 ///
 /// Prints verbose information and error messages.
-/// Returns an integer intended as a program return code.
+/// Returns a Result type with the Err variant containing the error message.
 ///
 /// # Parameters
 ///
 /// _files_ must all exist as non-directories.
 /// _target_dir_ must be a directory.
 ///
-fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i32 {
+fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> UResult<()> {
     if !target_dir.is_dir() {
-        show_error!("target '{}' is not a directory", target_dir.display());
+        return Err(InstallError::TargetDirIsntDir(target_dir.to_path_buf()).into());
-        return 1;
     }
-
-    let mut all_successful = true;
     for sourcepath in files.iter() {
         if !sourcepath.exists() {
-            show_error!(
+            let err = UIoError::new(
-                "cannot stat '{}': No such file or directory",
+                std::io::ErrorKind::NotFound,
-                sourcepath.display()
+                format!("cannot stat '{}'", sourcepath.display()),
             );
-
+            show!(err);
-            all_successful = false;
             continue;
         }
 
         if sourcepath.is_dir() {
-            show_error!("omitting directory '{}'", sourcepath.display());
+            let err = InstallError::OmittingDirectory(sourcepath.to_path_buf());
-            all_successful = false;
+            show!(err);
             continue;
         }
 
@@ -479,37 +543,18 @@ fn copy_files_into_dir(files: &[PathBuf], target_dir: &Path, b: &Behavior) -> i3
         let filename = sourcepath.components().last().unwrap();
         targetpath.push(filename);
 
-        if copy(sourcepath, &targetpath, b).is_err() {
+        show_if_err!(copy(sourcepath, &targetpath, b));
-            all_successful = false;
-        }
-    }
-    if all_successful {
-        0
-    } else {
-        1
-    }
-}
-
-/// Copy a file to another file.
-///
-/// Prints verbose information and error messages.
-/// Returns an integer intended as a program return code.
-///
-/// # Parameters
-///
-/// _file_ must exist as a non-directory.
-/// _target_ must be a non-directory
-///
-fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> i32 {
-    if copy(file, target, b).is_err() {
-        1
-    } else {
-        0
     }
+    // If the exit code was set, or show! has been called at least once
+    // (which sets the exit code as well), function execution will end after
+    // this return.
+    Ok(())
 }
 
 /// Copy one file to a new location, changing metadata.
 ///
+/// Returns a Result type with the Err variant containing the error message.
+///
 /// # Parameters
 ///
 /// _from_ must exist as a non-directory.
@@ -520,8 +565,8 @@ fn copy_file_to_file(file: &Path, target: &Path, b: &Behavior) -> i32 {
 /// If the copy system call fails, we print a verbose error and return an empty error value.
 ///
 #[allow(clippy::cognitive_complexity)]
-fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
+fn copy(from: &Path, to: &Path, b: &Behavior) -> UResult<()> {
-    if b.compare && !need_copy(from, to, b) {
+    if b.compare && !need_copy(from, to, b)? {
         return Ok(());
     }
     // Declare the path here as we may need it for the verbose output below.
@@ -536,13 +581,12 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
         if let Some(ref backup_path) = backup_path {
             // TODO!!
             if let Err(err) = fs::rename(to, backup_path) {
-                show_error!(
+                return Err(InstallError::BackupFailed(
-                    "install: cannot backup file '{}' to '{}': {}",
+                    to.to_path_buf(),
-                    to.display(),
+                    backup_path.to_path_buf(),
-                    backup_path.display(),
+                    err,
-                    err
+                )
-                );
+                .into());
-                return Err(());
             }
         }
     }
@@ -552,52 +596,41 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
          * https://github.com/rust-lang/rust/issues/79390
          */
         if let Err(err) = File::create(to) {
-            show_error!(
+            return Err(
-                "install: cannot install '{}' to '{}': {}",
+                InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into(),
-                from.display(),
-                to.display(),
-                err
             );
-            return Err(());
         }
     } else if let Err(err) = fs::copy(from, to) {
-        show_error!(
+        return Err(InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into());
-            "cannot install '{}' to '{}': {}",
-            from.display(),
-            to.display(),
-            err
-        );
-        return Err(());
     }
 
     if b.strip && cfg!(not(windows)) {
         match Command::new(&b.strip_program).arg(to).output() {
             Ok(o) => {
                 if !o.status.success() {
-                    crash!(
+                    return Err(InstallError::StripProgramFailed(
-                        1,
+                        String::from_utf8(o.stderr).unwrap_or_default(),
-                        "strip program failed: {}",
+                    )
-                        String::from_utf8(o.stderr).unwrap_or_default()
+                    .into());
-                    );
                 }
             }
-            Err(e) => crash!(1, "strip program execution failed: {}", e),
+            Err(e) => return Err(InstallError::StripProgramFailed(e.to_string()).into()),
         }
     }
 
     if mode::chmod(to, b.mode()).is_err() {
-        return Err(());
+        return Err(InstallError::ChmodFailed(to.to_path_buf()).into());
     }
 
     if !b.owner.is_empty() {
         let meta = match fs::metadata(to) {
             Ok(meta) => meta,
-            Err(f) => crash!(1, "{}", f.to_string()),
+            Err(e) => return Err(InstallError::MetadataFailed(e).into()),
         };
 
         let owner_id = match usr2uid(&b.owner) {
             Ok(g) => g,
-            _ => crash!(1, "no such user: {}", b.owner),
+            _ => return Err(InstallError::NoSuchUser(b.owner.clone()).into()),
         };
         let gid = meta.gid();
         match wrap_chown(
@@ -620,14 +653,14 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
     if !b.group.is_empty() {
         let meta = match fs::metadata(to) {
             Ok(meta) => meta,
-            Err(f) => crash!(1, "{}", f.to_string()),
+            Err(e) => return Err(InstallError::MetadataFailed(e).into()),
         };
 
         let group_id = match grp2gid(&b.group) {
             Ok(g) => g,
-            _ => crash!(1, "no such group: {}", b.group),
+            _ => return Err(InstallError::NoSuchGroup(b.group.clone()).into()),
         };
-        match wrap_chgrp(to, &meta, group_id, false, Verbosity::Normal) {
+        match wrap_chgrp(to, &meta, Some(group_id), false, Verbosity::Normal) {
             Ok(n) => {
                 if !n.is_empty() {
                     show_error!("{}", n);
@@ -640,7 +673,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
     if b.preserve_timestamps {
         let meta = match fs::metadata(from) {
             Ok(meta) => meta,
-            Err(f) => crash!(1, "{}", f.to_string()),
+            Err(e) => return Err(InstallError::MetadataFailed(e).into()),
         };
 
         let modified_time = FileTime::from_last_modification_time(&meta);
@@ -664,6 +697,7 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
 }
 
 /// Return true if a file is necessary to copy. This is the case when:
+///
 /// - _from_ or _to_ is nonexistent;
 /// - either file has a sticky bit or set[ug]id bit, or the user specified one;
 /// - either file isn't a regular file;
@@ -679,14 +713,14 @@ fn copy(from: &Path, to: &Path, b: &Behavior) -> Result<(), ()> {
 ///
 /// Crashes the program if a nonexistent owner or group is specified in _b_.
 ///
-fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool {
+fn need_copy(from: &Path, to: &Path, b: &Behavior) -> UResult<bool> {
     let from_meta = match fs::metadata(from) {
         Ok(meta) => meta,
-        Err(_) => return true,
+        Err(_) => return Ok(true),
     };
     let to_meta = match fs::metadata(to) {
         Ok(meta) => meta,
-        Err(_) => return true,
+        Err(_) => return Ok(true),
     };
 
     // setuid || setgid || sticky
@@ -696,15 +730,15 @@ fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool {
         || from_meta.mode() & extra_mode != 0
         || to_meta.mode() & extra_mode != 0
     {
-        return true;
+        return Ok(true);
     }
 
     if !from_meta.is_file() || !to_meta.is_file() {
-        return true;
+        return Ok(true);
     }
 
     if from_meta.len() != to_meta.len() {
-        return true;
+        return Ok(true);
     }
 
     // TODO: if -P (#1809) and from/to contexts mismatch, return true.
@@ -712,31 +746,31 @@ fn need_copy(from: &Path, to: &Path, b: &Behavior) -> bool {
     if !b.owner.is_empty() {
         let owner_id = match usr2uid(&b.owner) {
             Ok(id) => id,
-            _ => crash!(1, "no such user: {}", b.owner),
+            _ => return Err(InstallError::NoSuchUser(b.owner.clone()).into()),
         };
         if owner_id != to_meta.uid() {
-            return true;
+            return Ok(true);
         }
     } else if !b.group.is_empty() {
         let group_id = match grp2gid(&b.group) {
             Ok(id) => id,
-            _ => crash!(1, "no such group: {}", b.group),
+            _ => return Err(InstallError::NoSuchGroup(b.group.clone()).into()),
         };
         if group_id != to_meta.gid() {
-            return true;
+            return Ok(true);
         }
     } else {
         #[cfg(not(target_os = "windows"))]
         unsafe {
             if to_meta.uid() != geteuid() || to_meta.gid() != getegid() {
-                return true;
+                return Ok(true);
             }
         }
     }
 
     if !diff(from.to_str().unwrap(), to.to_str().unwrap()) {
-        return true;
+        return Ok(true);
     }
 
-    false
+    Ok(false)
 }

src/uu/ln/src/ln.rs

@@ -11,7 +11,7 @@
 extern crate uucore;
 
 use clap::{crate_version, App, Arg};
-use uucore::error::{UCustomError, UResult};
+use uucore::error::{UError, UResult};
 
 use std::borrow::Cow;
 use std::error::Error;
@@ -79,7 +79,7 @@ impl Display for LnError {
 
 impl Error for LnError {}
 
-impl UCustomError for LnError {
+impl UError for LnError {
     fn code(&self) -> i32 {
         match self {
             Self::TargetIsDirectory(_) => 1,

src/uu/ls/src/ls.rs

@@ -39,7 +39,7 @@ use std::{
     time::Duration,
 };
 use term_grid::{Cell, Direction, Filling, Grid, GridOptions};
-use uucore::error::{set_exit_code, FromIo, UCustomError, UResult};
+use uucore::error::{set_exit_code, FromIo, UError, UResult};
 
 use unicode_width::UnicodeWidthStr;
 #[cfg(unix)]
@@ -127,13 +127,15 @@ pub mod options {
     pub static IGNORE: &str = "ignore";
 }
 
+const DEFAULT_TERM_WIDTH: u16 = 80;
+
 #[derive(Debug)]
 enum LsError {
     InvalidLineWidth(String),
     NoMetadata(PathBuf),
 }
 
-impl UCustomError for LsError {
+impl UError for LsError {
     fn code(&self) -> i32 {
         match self {
             LsError::InvalidLineWidth(_) => 2,
@@ -229,7 +231,7 @@ struct Config {
     inode: bool,
     color: Option<LsColors>,
     long: LongFormat,
-    width: Option<u16>,
+    width: u16,
     quoting_style: QuotingStyle,
     indicator_style: IndicatorStyle,
     time_style: TimeStyle,
@@ -258,16 +260,20 @@ impl Config {
                     // below should never happen as clap already restricts the values.
                     _ => unreachable!("Invalid field for --format"),
                 },
-                options::FORMAT,
+                Some(options::FORMAT),
             )
         } else if options.is_present(options::format::LONG) {
-            (Format::Long, options::format::LONG)
+            (Format::Long, Some(options::format::LONG))
         } else if options.is_present(options::format::ACROSS) {
-            (Format::Across, options::format::ACROSS)
+            (Format::Across, Some(options::format::ACROSS))
         } else if options.is_present(options::format::COMMAS) {
-            (Format::Commas, options::format::COMMAS)
+            (Format::Commas, Some(options::format::COMMAS))
+        } else if options.is_present(options::format::COLUMNS) {
+            (Format::Columns, Some(options::format::COLUMNS))
+        } else if atty::is(atty::Stream::Stdout) {
+            (Format::Columns, None)
         } else {
-            (Format::Columns, options::format::COLUMNS)
+            (Format::OneLine, None)
         };
 
         // The -o, -n and -g options are tricky. They cannot override with each
@@ -286,9 +292,8 @@ impl Config {
         // options, but manually whether they have an index that's greater than
         // the other format options. If so, we set the appropriate format.
         if format != Format::Long {
-            let idx = options
+            let idx = opt
-                .indices_of(opt)
+                .and_then(|opt| options.indices_of(opt).map(|x| x.max().unwrap()))
-                .map(|x| x.max().unwrap())
                 .unwrap_or(0);
             if [
                 options::format::LONG_NO_OWNER,
@@ -399,10 +404,25 @@ impl Config {
 
         let width = match options.value_of(options::WIDTH) {
             Some(x) => match x.parse::<u16>() {
-                Ok(u) => Some(u),
+                Ok(u) => u,
                 Err(_) => return Err(LsError::InvalidLineWidth(x.into()).into()),
             },
-            None => termsize::get().map(|s| s.cols),
+            None => match termsize::get() {
+                Some(size) => size.cols,
+                None => match std::env::var("COLUMNS") {
+                    Ok(columns) => match columns.parse() {
+                        Ok(columns) => columns,
+                        Err(_) => {
+                            show_error!(
+                                "ignoring invalid width in environment variable COLUMNS: '{}'",
+                                columns
+                            );
+                            DEFAULT_TERM_WIDTH
+                        }
+                    },
+                    Err(_) => DEFAULT_TERM_WIDTH,
+                },
+            },
         };
 
         #[allow(clippy::needless_bool)]
@@ -1411,15 +1431,10 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter<Stdout
     } else {
         let names = items.iter().filter_map(|i| display_file_name(i, config));
 
-        match (&config.format, config.width) {
+        match config.format {
-            (Format::Columns, Some(width)) => {
+            Format::Columns => display_grid(names, config.width, Direction::TopToBottom, out),
-                display_grid(names, width, Direction::TopToBottom, out)
+            Format::Across => display_grid(names, config.width, Direction::LeftToRight, out),
-            }
+            Format::Commas => {
-            (Format::Across, Some(width)) => {
-                display_grid(names, width, Direction::LeftToRight, out)
-            }
-            (Format::Commas, width_opt) => {
-                let term_width = width_opt.unwrap_or(1);
                 let mut current_col = 0;
                 let mut names = names;
                 if let Some(name) = names.next() {
@@ -1428,7 +1443,8 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter<Stdout
                 }
                 for name in names {
                     let name_width = name.width as u16;
-                    if current_col + name_width + 1 > term_width {
+                    // If the width is 0 we print one single line
+                    if config.width != 0 && current_col + name_width + 1 > config.width {
                         current_col = name_width + 2;
                         let _ = write!(out, ",\n{}", name.contents);
                     } else {
@@ -1480,6 +1496,20 @@ fn display_grid(
     direction: Direction,
     out: &mut BufWriter<Stdout>,
 ) {
+    if width == 0 {
+        // If the width is 0 we print one single line
+        let mut printed_something = false;
+        for name in names {
+            if printed_something {
+                let _ = write!(out, "  ");
+            }
+            printed_something = true;
+            let _ = write!(out, "{}", name.contents);
+        }
+        if printed_something {
+            let _ = writeln!(out);
+        }
+    } else {
         let mut grid = Grid::new(GridOptions {
             filling: Filling::Spaces(2),
             direction,
@@ -1498,6 +1528,7 @@ fn display_grid(
                 let _ = write!(out, "{}", grid.fit_into_columns(1));
             }
         }
+    }
 }
 
 fn display_item_long(

src/uu/mktemp/src/mktemp.rs

@@ -12,7 +12,7 @@
 extern crate uucore;
 
 use clap::{crate_version, App, Arg};
-use uucore::error::{FromIo, UCustomError, UResult};
+use uucore::error::{FromIo, UError, UResult};
 
 use std::env;
 use std::error::Error;
@@ -49,7 +49,7 @@ enum MkTempError {
     InvalidTemplate(String),
 }
 
-impl UCustomError for MkTempError {}
+impl UError for MkTempError {}
 
 impl Error for MkTempError {}
 

src/uu/nice/src/nice.rs

@@ -10,21 +10,13 @@
 #[macro_use]
 extern crate uucore;
 
-use libc::{c_char, c_int, execvp};
+use libc::{c_char, c_int, execvp, PRIO_PROCESS};
 use std::ffi::CString;
 use std::io::Error;
 use std::ptr;
 
 use clap::{crate_version, App, AppSettings, Arg};
 
-// XXX: PRIO_PROCESS is 0 on at least FreeBSD and Linux.  Don't know about Mac OS X.
-const PRIO_PROCESS: c_int = 0;
-
-extern "C" {
-    fn getpriority(which: c_int, who: c_int) -> c_int;
-    fn setpriority(which: c_int, who: c_int, prio: c_int) -> c_int;
-}
-
 pub mod options {
     pub static ADJUSTMENT: &str = "adjustment";
     pub static COMMAND: &str = "COMMAND";
@@ -50,7 +42,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
     let mut niceness = unsafe {
         nix::errno::Errno::clear();
-        getpriority(PRIO_PROCESS, 0)
+        libc::getpriority(PRIO_PROCESS, 0)
     };
     if Error::last_os_error().raw_os_error().unwrap() != 0 {
         show_error!("getpriority: {}", Error::last_os_error());
@@ -84,7 +76,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     };
 
     niceness += adjustment;
-    if unsafe { setpriority(PRIO_PROCESS, 0, niceness) } == -1 {
+    if unsafe { libc::setpriority(PRIO_PROCESS, 0, niceness) } == -1 {
         show_warning!("setpriority: {}", Error::last_os_error());
     }
 

src/uu/od/src/od.rs

@@ -435,6 +435,7 @@ pub fn uu_app() -> clap::App<'static, 'static> {
                 .long(options::FORMAT)
                 .help("select output format or formats")
                 .multiple(true)
+                .number_of_values(1)
                 .value_name("TYPE"),
         )
         .arg(

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, UCustomError, UResult, USimpleError, UUsageError};
+use uucore::error::{set_exit_code, UError, UResult, USimpleError, UUsageError};
 use uucore::parse_size::{parse_size, ParseSizeError};
 use uucore::version_cmp::version_cmp;
 use uucore::InvalidEncodingHandling;
@@ -164,7 +164,7 @@ enum SortError {
 
 impl Error for SortError {}
 
-impl UCustomError for SortError {
+impl UError for SortError {
     fn code(&self) -> i32 {
         match self {
             SortError::Disorder { .. } => 1,
@@ -1238,7 +1238,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         if separator.len() != 1 {
             return Err(UUsageError::new(
                 2,
-                "separator must be exactly one character long".into(),
+                "separator must be exactly one character long",
             ));
         }
         settings.separator = Some(separator.chars().next().unwrap())
@@ -1517,7 +1517,7 @@ fn exec(
         file_merger.write_all(settings, output)
     } else if settings.check {
         if files.len() > 1 {
-            Err(UUsageError::new(2, "only one file allowed with -c".into()))
+            Err(UUsageError::new(2, "only one file allowed with -c"))
         } else {
             check::check(files.first().unwrap(), settings)
         }

src/uu/touch/src/touch.rs

@@ -17,7 +17,7 @@ use clap::{crate_version, App, Arg, ArgGroup};
 use filetime::*;
 use std::fs::{self, File};
 use std::path::Path;
-use uucore::error::{FromIo, UResult, USimpleError};
+use uucore::error::{FromIo, UError, UResult, USimpleError};
 
 static ABOUT: &str = "Update the access and modification times of each FILE to the current time.";
 pub mod options {

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

@@ -49,13 +49,14 @@ fn chgrp<P: AsRef<Path>>(path: P, gid: gid_t, follow: bool) -> IOResult<()> {
 pub fn wrap_chgrp<P: AsRef<Path>>(
     path: P,
     meta: &Metadata,
-    dest_gid: gid_t,
+    dest_gid: Option<gid_t>,
     follow: bool,
     verbosity: Verbosity,
 ) -> Result<String, String> {
     use self::Verbosity::*;
     let path = path.as_ref();
     let mut out: String = String::new();
+    let dest_gid = dest_gid.unwrap_or_else(|| meta.gid());
 
     if let Err(e) = chgrp(path, dest_gid, follow) {
         match verbosity {

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

@@ -6,11 +6,11 @@
 //! This module provides types to reconcile these exit codes with idiomatic Rust error
 //! handling. This has a couple advantages over manually using [`std::process::exit`]:
 //! 1. It enables the use of `?`, `map_err`, `unwrap_or`, etc. in `uumain`.
-//! 1. It encourages the use of `UResult`/`Result` in functions in the utils.
+//! 1. It encourages the use of [`UResult`]/[`Result`] in functions in the utils.
 //! 1. The error messages are largely standardized across utils.
 //! 1. Standardized error messages can be created from external result types
 //!    (i.e. [`std::io::Result`] & `clap::ClapResult`).
-//! 1. `set_exit_code` takes away the burden of manually tracking exit codes for non-fatal errors.
+//! 1. [`set_exit_code`] takes away the burden of manually tracking exit codes for non-fatal errors.
 //!
 //! # Usage
 //! The signature of a typical util should be:
@@ -19,7 +19,7 @@
 //!     ...
 //! }
 //! ```
-//! [`UResult`] is a simple wrapper around [`Result`] with a custom error type: [`UError`]. The
+//! [`UResult`] is a simple wrapper around [`Result`] with a custom error trait: [`UError`]. The
 //! most important difference with types implementing [`std::error::Error`] is that [`UError`]s
 //! can specify the exit code of the program when they are returned from `uumain`:
 //! * When `Ok` is returned, the code set with [`set_exit_code`] is used as exit code. If
@@ -41,13 +41,15 @@
 //! [`set_exit_code`]. See the documentation on that function for more information.
 //!
 //! # Guidelines
-//! * Use common errors where possible.
+//! * Use error types from `uucore` where possible.
-//! * Add variants to [`UCommonError`] if an error appears in multiple utils.
+//! * Add error types to `uucore` if an error appears in multiple utils.
 //! * Prefer proper custom error types over [`ExitCode`] and [`USimpleError`].
 //! * [`USimpleError`] may be used in small utils with simple error handling.
 //! * Using [`ExitCode`] is not recommended but can be useful for converting utils to use
 //!   [`UResult`].
 
+// spell-checker:ignore uioerror
+
 use std::{
     error::Error,
     fmt::{Display, Formatter},
@@ -85,115 +87,10 @@ pub fn set_exit_code(code: i32) {
     EXIT_CODE.store(code, Ordering::SeqCst);
 }
 
-/// Should be returned by all utils.
+/// Result type that should be returned by all utils.
-///
+pub type UResult<T> = Result<T, Box<dyn UError>>;
-/// Two additional methods are implemented on [`UResult`] on top of the normal [`Result`] methods:
-/// `map_err_code` & `map_err_code_message`.
-///
-/// These methods are used to convert [`UCommonError`]s into errors with a custom error code and
-/// message.
-pub type UResult<T> = Result<T, UError>;
 
-trait UResultTrait<T> {
+/// Custom errors defined by the utils and `uucore`.
-    fn map_err_code(self, mapper: fn(&UCommonError) -> Option<i32>) -> Self;
-    fn map_err_code_and_message(self, mapper: fn(&UCommonError) -> Option<(i32, String)>) -> Self;
-}
-
-impl<T> UResultTrait<T> for UResult<T> {
-    fn map_err_code(self, mapper: fn(&UCommonError) -> Option<i32>) -> Self {
-        if let Err(UError::Common(error)) = self {
-            if let Some(code) = mapper(&error) {
-                Err(UCommonErrorWithCode { code, error }.into())
-            } else {
-                Err(error.into())
-            }
-        } else {
-            self
-        }
-    }
-
-    fn map_err_code_and_message(self, mapper: fn(&UCommonError) -> Option<(i32, String)>) -> Self {
-        if let Err(UError::Common(ref error)) = self {
-            if let Some((code, message)) = mapper(error) {
-                return Err(USimpleError { code, message }.into());
-            }
-        }
-        self
-    }
-}
-
-/// The error type of [`UResult`].
-///
-/// `UError::Common` errors are defined in [`uucore`](crate) while `UError::Custom` errors are
-/// defined by the utils.
-/// ```
-/// use uucore::error::USimpleError;
-/// let err = USimpleError::new(1, "Error!!".into());
-/// assert_eq!(1, err.code());
-/// assert_eq!(String::from("Error!!"), format!("{}", err));
-/// ```
-pub enum UError {
-    Common(UCommonError),
-    Custom(Box<dyn UCustomError>),
-}
-
-impl UError {
-    /// The error code of [`UResult`]
-    ///
-    /// This function defines the error code associated with an instance of
-    /// [`UResult`]. To associate error codes for self-defined instances of
-    /// `UResult::Custom` (i.e. [`UCustomError`]), implement the
-    /// [`code`-function there](UCustomError::code).
-    pub fn code(&self) -> i32 {
-        match self {
-            UError::Common(e) => e.code(),
-            UError::Custom(e) => e.code(),
-        }
-    }
-
-    /// Whether to print usage help for a [`UResult`]
-    ///
-    /// Defines if a variant of [`UResult`] should print a short usage message
-    /// below the error. The usage message is printed when this function returns
-    /// `true`. To do this for self-defined instances of `UResult::Custom` (i.e.
-    /// [`UCustomError`]), implement the [`usage`-function
-    /// there](UCustomError::usage).
-    pub fn usage(&self) -> bool {
-        match self {
-            UError::Common(e) => e.usage(),
-            UError::Custom(e) => e.usage(),
-        }
-    }
-}
-
-impl From<UCommonError> for UError {
-    fn from(v: UCommonError) -> Self {
-        UError::Common(v)
-    }
-}
-
-impl From<i32> for UError {
-    fn from(v: i32) -> Self {
-        UError::Custom(Box::new(ExitCode(v)))
-    }
-}
-
-impl<E: UCustomError + 'static> From<E> for UError {
-    fn from(v: E) -> Self {
-        UError::Custom(Box::new(v) as Box<dyn UCustomError>)
-    }
-}
-
-impl Display for UError {
-    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
-        match self {
-            UError::Common(e) => e.fmt(f),
-            UError::Custom(e) => e.fmt(f),
-        }
-    }
-}
-
-/// Custom errors defined by the utils.
 ///
 /// All errors should implement [`std::error::Error`], [`std::fmt::Display`] and
 /// [`std::fmt::Debug`] and have an additional `code` method that specifies the
@@ -202,7 +99,7 @@ impl Display for UError {
 /// An example of a custom error from `ls`:
 ///
 /// ```
-/// use uucore::error::{UCustomError, UResult};
+/// use uucore::error::{UError, UResult};
 /// use std::{
 ///     error::Error,
 ///     fmt::{Display, Debug},
@@ -215,7 +112,7 @@ impl Display for UError {
 ///     NoMetadata(PathBuf),
 /// }
 ///
-/// impl UCustomError for LsError {
+/// impl UError for LsError {
 ///     fn code(&self) -> i32 {
 ///         match self {
 ///             LsError::InvalidLineWidth(_) => 2,
@@ -246,12 +143,12 @@ impl Display for UError {
 /// }
 /// ```
 ///
-/// The call to `into()` is required to convert the [`UCustomError`] to an
+/// The call to `into()` is required to convert the `LsError` to
-/// instance of [`UError`].
+/// [`Box<dyn UError>`]. The implementation for `From` is provided automatically.
 ///
 /// A crate like [`quick_error`](https://crates.io/crates/quick-error) might
 /// also be used, but will still require an `impl` for the `code` method.
-pub trait UCustomError: Error + Send {
+pub trait UError: Error + Send {
     /// Error code of a custom error.
     ///
     /// Set a return value for each variant of an enum-type to associate an
@@ -261,7 +158,7 @@ pub trait UCustomError: Error + Send {
     /// # Example
     ///
     /// ```
-    /// use uucore::error::{UCustomError};
+    /// use uucore::error::{UError};
     /// use std::{
     ///     error::Error,
     ///     fmt::{Display, Debug},
@@ -275,7 +172,7 @@ pub trait UCustomError: Error + Send {
     ///     Bing(),
     /// }
     ///
-    /// impl UCustomError for MyError {
+    /// impl UError for MyError {
     ///     fn code(&self) -> i32 {
     ///         match self {
     ///             MyError::Foo(_) => 2,
@@ -312,7 +209,7 @@ pub trait UCustomError: Error + Send {
     /// # Example
     ///
     /// ```
-    /// use uucore::error::{UCustomError};
+    /// use uucore::error::{UError};
     /// use std::{
     ///     error::Error,
     ///     fmt::{Display, Debug},
@@ -326,7 +223,7 @@ pub trait UCustomError: Error + Send {
     ///     Bing(),
     /// }
     ///
-    /// impl UCustomError for MyError {
+    /// impl UError for MyError {
     ///     fn usage(&self) -> bool {
     ///         match self {
     ///             // This will have a short usage help appended
@@ -355,47 +252,23 @@ pub trait UCustomError: Error + Send {
     }
 }
 
-impl From<Box<dyn UCustomError>> for i32 {
+impl<T> From<T> for Box<dyn UError>
-    fn from(e: Box<dyn UCustomError>) -> i32 {
+where
-        e.code()
+    T: UError + 'static,
+{
+    fn from(t: T) -> Box<dyn UError> {
+        Box::new(t)
     }
 }
 
-/// A [`UCommonError`] with an overridden exit code.
+/// A simple error type with an exit code and a message that implements [`UError`].
 ///
-/// This exit code is returned instead of the default exit code for the [`UCommonError`]. This is
-/// typically created with the either the `UResult::map_err_code` or `UCommonError::with_code`
-/// method.
-#[derive(Debug)]
-pub struct UCommonErrorWithCode {
-    code: i32,
-    error: UCommonError,
-}
-
-impl Error for UCommonErrorWithCode {}
-
-impl Display for UCommonErrorWithCode {
-    fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> {
-        self.error.fmt(f)
-    }
-}
-
-impl UCustomError for UCommonErrorWithCode {
-    fn code(&self) -> i32 {
-        self.code
-    }
-}
-
-/// A simple error type with an exit code and a message that implements [`UCustomError`].
-///
-/// It is typically created with the `UResult::map_err_code_and_message` method. Alternatively, it
-/// can be constructed by manually:
 /// ```
 /// use uucore::error::{UResult, USimpleError};
 /// let err = USimpleError { code: 1, message: "error!".into()};
 /// let res: UResult<()> = Err(err.into());
 /// // or using the `new` method:
-/// let res: UResult<()> = Err(USimpleError::new(1, "error!".into()));
+/// let res: UResult<()> = Err(USimpleError::new(1, "error!"));
 /// ```
 #[derive(Debug)]
 pub struct USimpleError {
@@ -405,8 +278,11 @@ pub struct USimpleError {
 
 impl USimpleError {
     #[allow(clippy::new_ret_no_self)]
-    pub fn new(code: i32, message: String) -> UError {
+    pub fn new<S: Into<String>>(code: i32, message: S) -> Box<dyn UError> {
-        UError::Custom(Box::new(Self { code, message }))
+        Box::new(Self {
+            code,
+            message: message.into(),
+        })
     }
 }
 
@@ -418,7 +294,7 @@ impl Display for USimpleError {
     }
 }
 
-impl UCustomError for USimpleError {
+impl UError for USimpleError {
     fn code(&self) -> i32 {
         self.code
     }
@@ -432,8 +308,11 @@ pub struct UUsageError {
 
 impl UUsageError {
     #[allow(clippy::new_ret_no_self)]
-    pub fn new(code: i32, message: String) -> UError {
+    pub fn new<S: Into<String>>(code: i32, message: S) -> Box<dyn UError> {
-        UError::Custom(Box::new(Self { code, message }))
+        Box::new(Self {
+            code,
+            message: message.into(),
+        })
     }
 }
 
@@ -445,7 +324,7 @@ impl Display for UUsageError {
     }
 }
 
-impl UCustomError for UUsageError {
+impl UError for UUsageError {
     fn code(&self) -> i32 {
         self.code
     }
@@ -463,13 +342,13 @@ impl UCustomError for UUsageError {
 /// There are two ways to construct this type: with [`UIoError::new`] or by calling the
 /// [`FromIo::map_err_context`] method on a [`std::io::Result`] or [`std::io::Error`].
 /// ```
-/// use uucore::error::{FromIo, UResult, UIoError, UCommonError};
+/// use uucore::error::{FromIo, UResult, UIoError, UError};
 /// use std::fs::File;
 /// use std::path::Path;
 /// let path = Path::new("test.txt");
 ///
 /// // Manual construction
-/// let e: UIoError = UIoError::new(
+/// let e: Box<dyn UError> = UIoError::new(
 ///     std::io::ErrorKind::NotFound,
 ///     format!("cannot access '{}'", path.display())
 /// );
@@ -485,22 +364,17 @@ pub struct UIoError {
 }
 
 impl UIoError {
-    pub fn new(kind: std::io::ErrorKind, context: String) -> Self {
+    #[allow(clippy::new_ret_no_self)]
-        Self {
+    pub fn new<S: Into<String>>(kind: std::io::ErrorKind, context: S) -> Box<dyn UError> {
-            context,
+        Box::new(Self {
+            context: context.into(),
             inner: std::io::Error::new(kind, ""),
-        }
+        })
-    }
-
-    pub fn code(&self) -> i32 {
-        1
-    }
-
-    pub fn usage(&self) -> bool {
-        false
     }
 }
 
+impl UError for UIoError {}
+
 impl Error for UIoError {}
 
 impl Display for UIoError {
@@ -535,89 +409,102 @@ impl Display for UIoError {
     }
 }
 
-/// Enables the conversion from `std::io::Error` to `UError` and from `std::io::Result` to
+/// Enables the conversion from [`std::io::Error`] to [`UError`] and from [`std::io::Result`] to
-/// `UResult`.
+/// [`UResult`].
 pub trait FromIo<T> {
     fn map_err_context(self, context: impl FnOnce() -> String) -> T;
 }
 
-impl FromIo<UIoError> for std::io::Error {
+impl FromIo<Box<UIoError>> for std::io::Error {
-    fn map_err_context(self, context: impl FnOnce() -> String) -> UIoError {
+    fn map_err_context(self, context: impl FnOnce() -> String) -> Box<UIoError> {
-        UIoError {
+        Box::new(UIoError {
             context: (context)(),
             inner: self,
-        }
+        })
     }
 }
 
 impl<T> FromIo<UResult<T>> for std::io::Result<T> {
     fn map_err_context(self, context: impl FnOnce() -> String) -> UResult<T> {
-        self.map_err(|e| UError::Common(UCommonError::Io(e.map_err_context(context))))
+        self.map_err(|e| e.map_err_context(context) as Box<dyn UError>)
     }
 }
 
-impl FromIo<UIoError> for std::io::ErrorKind {
+impl FromIo<Box<UIoError>> for std::io::ErrorKind {
-    fn map_err_context(self, context: impl FnOnce() -> String) -> UIoError {
+    fn map_err_context(self, context: impl FnOnce() -> String) -> Box<UIoError> {
-        UIoError {
+        Box::new(UIoError {
             context: (context)(),
             inner: std::io::Error::new(self, ""),
-        }
+        })
     }
 }
 
-impl From<UIoError> for UCommonError {
+/// Shorthand to construct [`UIoError`]-instances.
-    fn from(e: UIoError) -> UCommonError {
-        UCommonError::Io(e)
-    }
-}
-
-impl From<UIoError> for UError {
-    fn from(e: UIoError) -> UError {
-        let common: UCommonError = e.into();
-        common.into()
-    }
-}
-
-/// Common errors for utilities.
 ///
-/// If identical errors appear across multiple utilities, they should be added here.
+/// This macro serves as a convenience call to quickly construct instances of
-#[derive(Debug)]
+/// [`UIoError`]. It takes:
-pub enum UCommonError {
+///
-    Io(UIoError),
+/// - An instance of [`std::io::Error`]
-    // Clap(UClapError),
+/// - A `format!`-compatible string and
-}
+/// - An arbitrary number of arguments to the format string
-
+///
-impl UCommonError {
+/// In exactly this order. It is equivalent to the more verbose code seen in the
-    pub fn with_code(self, code: i32) -> UCommonErrorWithCode {
+/// example.
-        UCommonErrorWithCode { code, error: self }
+///
-    }
+/// # Examples
-
+///
-    pub fn code(&self) -> i32 {
+/// ```
-        1
+/// use uucore::error::UIoError;
-    }
+/// use uucore::uio_error;
-
+///
-    pub fn usage(&self) -> bool {
+/// let io_err = std::io::Error::new(
-        false
+///     std::io::ErrorKind::PermissionDenied, "fix me please!"
-    }
+/// );
-}
+///
-
+/// let uio_err = UIoError::new(
-impl From<UCommonError> for i32 {
+///     io_err.kind(),
-    fn from(common: UCommonError) -> i32 {
+///     format!("Error code: {}", 2)
-        match common {
+/// );
-            UCommonError::Io(e) => e.code(),
+///
-        }
+/// let other_uio_err = uio_error!(io_err, "Error code: {}", 2);
-    }
+///
-}
+/// // prints "fix me please!: Permission denied"
-
+/// println!("{}", uio_err);
-impl Error for UCommonError {}
+/// // prints "Error code: 2: Permission denied"
-
+/// println!("{}", other_uio_err);
-impl Display for UCommonError {
+/// ```
-    fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), std::fmt::Error> {
+///
-        match self {
+/// The [`std::fmt::Display`] impl of [`UIoError`] will then ensure that an
-            UCommonError::Io(e) => e.fmt(f),
+/// appropriate error message relating to the actual error kind of the
-        }
+/// [`std::io::Error`] is appended to whatever error message is defined in
-    }
+/// addition (as secondary argument).
-}
+///
+/// If you want to show only the error message for the [`std::io::ErrorKind`]
+/// that's contained in [`UIoError`], pass the second argument as empty string:
+///
+/// ```
+/// use uucore::error::UIoError;
+/// use uucore::uio_error;
+///
+/// let io_err = std::io::Error::new(
+///     std::io::ErrorKind::PermissionDenied, "fix me please!"
+/// );
+///
+/// let other_uio_err = uio_error!(io_err, "");
+///
+/// // prints: ": Permission denied"
+/// println!("{}", other_uio_err);
+/// ```
+//#[macro_use]
+#[macro_export]
+macro_rules! uio_error(
+    ($err:expr, $($args:tt)+) => ({
+        UIoError::new(
+            $err.kind(),
+            format!($($args)+)
+        )
+    })
+);
 
 /// A special error type that does not print any message when returned from
 /// `uumain`. Especially useful for porting utilities to using [`UResult`].
@@ -636,6 +523,13 @@ impl Display for UCommonError {
 #[derive(Debug)]
 pub struct ExitCode(pub i32);
 
+impl ExitCode {
+    #[allow(clippy::new_ret_no_self)]
+    pub fn new(code: i32) -> Box<dyn UError> {
+        Box::new(Self(code))
+    }
+}
+
 impl Error for ExitCode {}
 
 impl Display for ExitCode {
@@ -644,8 +538,14 @@ impl Display for ExitCode {
     }
 }
 
-impl UCustomError for ExitCode {
+impl UError for ExitCode {
     fn code(&self) -> i32 {
         self.0
     }
 }
+
+impl From<i32> for Box<dyn UError> {
+    fn from(i: i32) -> Self {
+        ExitCode::new(i)
+    }
+}

tests/by-util/test_cat.rs

@@ -1,5 +1,4 @@
 use crate::common::util::*;
-#[cfg(unix)]
 use std::fs::OpenOptions;
 #[cfg(unix)]
 use std::io::Read;
@@ -274,6 +273,26 @@ fn test_stdin_show_ends() {
     }
 }
 
+#[test]
+fn squeeze_all_files() {
+    // empty lines at the end of a file are "squeezed" together with empty lines at the beginning
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.write("input1", "a\n\n");
+    at.write("input2", "\n\nb");
+    ucmd.args(&["input1", "input2", "-s"])
+        .succeeds()
+        .stdout_only("a\n\nb");
+}
+
+#[test]
+fn test_show_ends_crlf() {
+    new_ucmd!()
+        .arg("-E")
+        .pipe_in("a\nb\r\n\rc\n\r\n\r")
+        .succeeds()
+        .stdout_only("a$\nb^M$\n\rc$\n^M$\n\r");
+}
+
 #[test]
 fn test_stdin_show_all() {
     for same_param in &["-A", "--show-all"] {
@@ -443,3 +462,49 @@ fn test_domain_socket() {
 
     thread.join().unwrap();
 }
+
+#[test]
+fn test_write_to_self_empty() {
+    // it's ok if the input file is also the output file if it's empty
+    let s = TestScenario::new(util_name!());
+    let file_path = s.fixtures.plus("file.txt");
+
+    let file = OpenOptions::new()
+        .create_new(true)
+        .write(true)
+        .append(true)
+        .open(&file_path)
+        .unwrap();
+
+    s.ucmd().set_stdout(file).arg(&file_path).succeeds();
+}
+
+#[test]
+fn test_write_to_self() {
+    let s = TestScenario::new(util_name!());
+    let file_path = s.fixtures.plus("first_file");
+    s.fixtures.write("second_file", "second_file_content.");
+
+    let file = OpenOptions::new()
+        .create_new(true)
+        .write(true)
+        .append(true)
+        .open(&file_path)
+        .unwrap();
+
+    s.fixtures.append("first_file", "first_file_content.");
+
+    s.ucmd()
+        .set_stdout(file)
+        .arg("first_file")
+        .arg("first_file")
+        .arg("second_file")
+        .fails()
+        .code_is(2)
+        .stderr_only("cat: first_file: input file is output file\ncat: first_file: input file is output file");
+
+    assert_eq!(
+        s.fixtures.read("first_file"),
+        "first_file_content.second_file_content."
+    );
+}

tests/by-util/test_chgrp.rs

@@ -244,3 +244,10 @@ fn basic_succeeds() {
             .no_stderr();
     }
 }
+
+#[test]
+fn test_no_change() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    at.touch("file");
+    ucmd.arg("").arg(at.plus("file")).succeeds();
+}

tests/by-util/test_cp.rs

@@ -16,8 +16,6 @@ use std::os::windows::fs::symlink_file;
 use filetime::FileTime;
 #[cfg(target_os = "linux")]
 use rlimit::Resource;
-#[cfg(not(windows))]
-use std::env;
 #[cfg(target_os = "linux")]
 use std::fs as std_fs;
 #[cfg(target_os = "linux")]
@@ -743,20 +741,16 @@ fn test_cp_deref_folder_to_folder() {
     let scene = TestScenario::new(util_name!());
     let at = &scene.fixtures;
 
-    let cwd = env::current_dir().unwrap();
+    let path_to_new_symlink = at.plus(TEST_COPY_FROM_FOLDER);
 
-    let path_to_new_symlink = at.subdir.join(TEST_COPY_FROM_FOLDER);
+    at.symlink_file(
-
+        &path_to_new_symlink
-    // Change the cwd to have a correct symlink
+            .join(TEST_HELLO_WORLD_SOURCE)
-    assert!(env::set_current_dir(&path_to_new_symlink).is_ok());
+            .to_string_lossy(),
-
+        &path_to_new_symlink
-    #[cfg(not(windows))]
+            .join(TEST_HELLO_WORLD_SOURCE_SYMLINK)
-    let _r = fs::symlink(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK);
+            .to_string_lossy(),
-    #[cfg(windows)]
+    );
-    let _r = symlink_file(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK);
-
-    // Back to the initial cwd (breaks the other tests)
-    assert!(env::set_current_dir(&cwd).is_ok());
 
     //using -P -R option
     scene
@@ -843,20 +837,16 @@ fn test_cp_no_deref_folder_to_folder() {
     let scene = TestScenario::new(util_name!());
     let at = &scene.fixtures;
 
-    let cwd = env::current_dir().unwrap();
+    let path_to_new_symlink = at.plus(TEST_COPY_FROM_FOLDER);
 
-    let path_to_new_symlink = at.subdir.join(TEST_COPY_FROM_FOLDER);
+    at.symlink_file(
-
+        &path_to_new_symlink
-    // Change the cwd to have a correct symlink
+            .join(TEST_HELLO_WORLD_SOURCE)
-    assert!(env::set_current_dir(&path_to_new_symlink).is_ok());
+            .to_string_lossy(),
-
+        &path_to_new_symlink
-    #[cfg(not(windows))]
+            .join(TEST_HELLO_WORLD_SOURCE_SYMLINK)
-    let _r = fs::symlink(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK);
+            .to_string_lossy(),
-    #[cfg(windows)]
+    );
-    let _r = symlink_file(TEST_HELLO_WORLD_SOURCE, TEST_HELLO_WORLD_SOURCE_SYMLINK);
-
-    // Back to the initial cwd (breaks the other tests)
-    assert!(env::set_current_dir(&cwd).is_ok());
 
     //using -P -R option
     scene
@@ -969,10 +959,9 @@ fn test_cp_archive() {
 }
 
 #[test]
-#[cfg(target_os = "unix")]
+#[cfg(unix)]
 fn test_cp_archive_recursive() {
     let (at, mut ucmd) = at_and_ucmd!();
-    let cwd = env::current_dir().unwrap();
 
     // creates
     // dir/1
@@ -988,26 +977,13 @@ fn test_cp_archive_recursive() {
     at.touch(&file_1.to_string_lossy());
     at.touch(&file_2.to_string_lossy());
 
-    // Change the cwd to have a correct symlink
+    at.symlink_file("1", &file_1_link.to_string_lossy());
-    assert!(env::set_current_dir(&at.subdir.join(TEST_COPY_TO_FOLDER)).is_ok());
+    at.symlink_file("2", &file_2_link.to_string_lossy());
-
-    #[cfg(not(windows))]
-    {
-        let _r = fs::symlink("1", &file_1_link);
-        let _r = fs::symlink("2", &file_2_link);
-    }
-    #[cfg(windows)]
-    {
-        let _r = symlink_file("1", &file_1_link);
-        let _r = symlink_file("2", &file_2_link);
-    }
-    // Back to the initial cwd (breaks the other tests)
-    assert!(env::set_current_dir(&cwd).is_ok());
 
     ucmd.arg("--archive")
         .arg(TEST_COPY_TO_FOLDER)
         .arg(TEST_COPY_TO_FOLDER_NEW)
-        .fails(); // fails for now
+        .succeeds();
 
     let scene2 = TestScenario::new("ls");
     let result = scene2
@@ -1025,18 +1001,6 @@ fn test_cp_archive_recursive() {
         .run();
 
     println!("ls dest {}", result.stdout_str());
-    assert!(at.file_exists(
-        &at.subdir
-            .join(TEST_COPY_TO_FOLDER_NEW)
-            .join("1.link")
-            .to_string_lossy()
-    ));
-    assert!(at.file_exists(
-        &at.subdir
-            .join(TEST_COPY_TO_FOLDER_NEW)
-            .join("2.link")
-            .to_string_lossy()
-    ));
     assert!(at.file_exists(
         &at.subdir
             .join(TEST_COPY_TO_FOLDER_NEW)

tests/by-util/test_factor.rs

@@ -8,7 +8,10 @@
 
 // spell-checker:ignore (methods) hexdigest
 
+use tempfile::TempDir;
+
 use crate::common::util::*;
+use std::fs::OpenOptions;
 use std::time::SystemTime;
 
 #[path = "../../src/uu/factor/sieve.rs"]
@@ -24,6 +27,43 @@ use self::sieve::Sieve;
 const NUM_PRIMES: usize = 10000;
 const NUM_TESTS: usize = 100;
 
+#[test]
+fn test_parallel() {
+    // factor should only flush the buffer at line breaks
+    let n_integers = 100_000;
+    let mut input_string = String::new();
+    for i in 0..=n_integers {
+        input_string.push_str(&(format!("{} ", i))[..]);
+    }
+
+    let tmp_dir = TempDir::new().unwrap();
+    let tmp_dir = AtPath::new(tmp_dir.path());
+    tmp_dir.touch("output");
+    let output = OpenOptions::new()
+        .append(true)
+        .open(tmp_dir.plus("output"))
+        .unwrap();
+
+    for mut child in (0..10)
+        .map(|_| {
+            new_ucmd!()
+                .set_stdout(output.try_clone().unwrap())
+                .pipe_in(input_string.clone())
+                .run_no_wait()
+        })
+        .collect::<Vec<_>>()
+    {
+        assert_eq!(child.wait().unwrap().code().unwrap(), 0);
+    }
+
+    let result = TestScenario::new(util_name!())
+        .ccmd("sort")
+        .arg(tmp_dir.plus("output"))
+        .succeeds();
+    let hash_check = sha1::Sha1::from(result.stdout()).hexdigest();
+    assert_eq!(hash_check, "cc743607c0ff300ff575d92f4ff0c87d5660c393");
+}
+
 #[test]
 fn test_first_100000_integers() {
     extern crate sha1;

tests/by-util/test_ls.rs

@@ -128,6 +128,7 @@ fn test_ls_width() {
         scene
             .ucmd()
             .args(&option.split(' ').collect::<Vec<_>>())
+            .arg("-C")
             .succeeds()
             .stdout_only("test-width-1  test-width-2  test-width-3  test-width-4\n");
     }
@@ -136,30 +137,33 @@ fn test_ls_width() {
         scene
             .ucmd()
             .args(&option.split(' ').collect::<Vec<_>>())
+            .arg("-C")
             .succeeds()
             .stdout_only("test-width-1  test-width-3\ntest-width-2  test-width-4\n");
     }
 
-    for option in &[
+    for option in &["-w 25", "-w=25", "--width=25", "--width 25"] {
-        "-w 25",
-        "-w=25",
-        "--width=25",
-        "--width 25",
-        "-w 0",
-        "-w=0",
-        "--width=0",
-        "--width 0",
-    ] {
         scene
             .ucmd()
             .args(&option.split(' ').collect::<Vec<_>>())
+            .arg("-C")
             .succeeds()
             .stdout_only("test-width-1\ntest-width-2\ntest-width-3\ntest-width-4\n");
     }
 
+    for option in &["-w 0", "-w=0", "--width=0", "--width 0"] {
+        scene
+            .ucmd()
+            .args(&option.split(' ').collect::<Vec<_>>())
+            .arg("-C")
+            .succeeds()
+            .stdout_only("test-width-1  test-width-2  test-width-3  test-width-4\n");
+    }
+
     scene
         .ucmd()
         .arg("-w=bad")
+        .arg("-C")
         .fails()
         .stderr_contains("invalid line width");
 
@@ -167,6 +171,7 @@ fn test_ls_width() {
         scene
             .ucmd()
             .args(&option.split(' ').collect::<Vec<_>>())
+            .arg("-C")
             .fails()
             .stderr_only("ls: invalid line width: '1a'");
     }
@@ -184,16 +189,10 @@ fn test_ls_columns() {
     // Columns is the default
     let result = scene.ucmd().succeeds();
 
-    #[cfg(not(windows))]
     result.stdout_only("test-columns-1\ntest-columns-2\ntest-columns-3\ntest-columns-4\n");
-    #[cfg(windows)]
-    result.stdout_only("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n");
 
     for option in &["-C", "--format=columns"] {
         let result = scene.ucmd().arg(option).succeeds();
-        #[cfg(not(windows))]
-        result.stdout_only("test-columns-1\ntest-columns-2\ntest-columns-3\ntest-columns-4\n");
-        #[cfg(windows)]
         result.stdout_only("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n");
     }
 
@@ -205,6 +204,38 @@ fn test_ls_columns() {
             .succeeds()
             .stdout_only("test-columns-1  test-columns-3\ntest-columns-2  test-columns-4\n");
     }
+
+    // On windows we are always able to get the terminal size, so we can't simulate falling back to the
+    // environment variable.
+    #[cfg(not(windows))]
+    {
+        for option in &["-C", "--format=columns"] {
+            scene
+                .ucmd()
+                .env("COLUMNS", "40")
+                .arg(option)
+                .succeeds()
+                .stdout_only("test-columns-1  test-columns-3\ntest-columns-2  test-columns-4\n");
+        }
+
+        scene
+            .ucmd()
+            .env("COLUMNS", "garbage")
+            .arg("-C")
+            .succeeds()
+            .stdout_is("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n")
+            .stderr_is("ls: ignoring invalid width in environment variable COLUMNS: 'garbage'");
+    }
+    scene
+        .ucmd()
+        .arg("-Cw0")
+        .succeeds()
+        .stdout_only("test-columns-1  test-columns-2  test-columns-3  test-columns-4\n");
+    scene
+        .ucmd()
+        .arg("-mw0")
+        .succeeds()
+        .stdout_only("test-columns-1, test-columns-2, test-columns-3, test-columns-4\n");
 }
 
 #[test]
@@ -220,12 +251,8 @@ fn test_ls_across() {
         let result = scene.ucmd().arg(option).succeeds();
         // Because the test terminal has width 0, this is the same output as
         // the columns option.
-        if cfg!(unix) {
-            result.stdout_only("test-across-1\ntest-across-2\ntest-across-3\ntest-across-4\n");
-        } else {
         result.stdout_only("test-across-1  test-across-2  test-across-3  test-across-4\n");
     }
-    }
 
     for option in &["-x", "--format=across"] {
         // Because the test terminal has width 0, this is the same output as
@@ -250,12 +277,8 @@ fn test_ls_commas() {
 
     for option in &["-m", "--format=commas"] {
         let result = scene.ucmd().arg(option).succeeds();
-        if cfg!(unix) {
-            result.stdout_only("test-commas-1,\ntest-commas-2,\ntest-commas-3,\ntest-commas-4\n");
-        } else {
         result.stdout_only("test-commas-1, test-commas-2, test-commas-3, test-commas-4\n");
     }
-    }
 
     for option in &["-m", "--format=commas"] {
         scene
@@ -571,13 +594,11 @@ fn test_ls_sort_name() {
     at.touch("test-1");
     at.touch("test-2");
 
-    let sep = if cfg!(unix) { "\n" } else { "  " };
-
     scene
         .ucmd()
         .arg("--sort=name")
         .succeeds()
-        .stdout_is(["test-1", "test-2", "test-3\n"].join(sep));
+        .stdout_is("test-1\ntest-2\ntest-3\n");
 
     let scene_dot = TestScenario::new(util_name!());
     let at = &scene_dot.fixtures;
@@ -591,7 +612,7 @@ fn test_ls_sort_name() {
         .arg("--sort=name")
         .arg("-A")
         .succeeds()
-        .stdout_is([".a", ".b", "a", "b\n"].join(sep));
+        .stdout_is(".a\n.b\na\nb\n");
 }
 
 #[test]
@@ -612,28 +633,16 @@ fn test_ls_order_size() {
     scene.ucmd().arg("-al").succeeds();
 
     let result = scene.ucmd().arg("-S").succeeds();
-    #[cfg(not(windows))]
     result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
-    #[cfg(windows)]
-    result.stdout_only("test-4  test-3  test-2  test-1\n");
 
     let result = scene.ucmd().arg("-S").arg("-r").succeeds();
-    #[cfg(not(windows))]
     result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
-    #[cfg(windows)]
-    result.stdout_only("test-1  test-2  test-3  test-4\n");
 
     let result = scene.ucmd().arg("--sort=size").succeeds();
-    #[cfg(not(windows))]
     result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
-    #[cfg(windows)]
-    result.stdout_only("test-4  test-3  test-2  test-1\n");
 
     let result = scene.ucmd().arg("--sort=size").arg("-r").succeeds();
-    #[cfg(not(windows))]
     result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
-    #[cfg(windows)]
-    result.stdout_only("test-1  test-2  test-3  test-4\n");
 }
 
 #[test]
@@ -755,9 +764,6 @@ fn test_ls_styles() {
 
     at.touch("test2");
     let result = scene.ucmd().arg("--full-time").arg("-x").succeeds();
-    #[cfg(not(windows))]
-    assert_eq!(result.stdout_str(), "test\ntest2\n");
-    #[cfg(windows)]
     assert_eq!(result.stdout_str(), "test  test2\n");
 }
 
@@ -794,28 +800,16 @@ fn test_ls_order_time() {
 
     // ctime was changed at write, so the order is 4 3 2 1
     let result = scene.ucmd().arg("-t").succeeds();
-    #[cfg(not(windows))]
     result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
-    #[cfg(windows)]
-    result.stdout_only("test-4  test-3  test-2  test-1\n");
 
     let result = scene.ucmd().arg("--sort=time").succeeds();
-    #[cfg(not(windows))]
     result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
-    #[cfg(windows)]
-    result.stdout_only("test-4  test-3  test-2  test-1\n");
 
     let result = scene.ucmd().arg("-tr").succeeds();
-    #[cfg(not(windows))]
     result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
-    #[cfg(windows)]
-    result.stdout_only("test-1  test-2  test-3  test-4\n");
 
     let result = scene.ucmd().arg("--sort=time").arg("-r").succeeds();
-    #[cfg(not(windows))]
     result.stdout_only("test-1\ntest-2\ntest-3\ntest-4\n");
-    #[cfg(windows)]
-    result.stdout_only("test-1  test-2  test-3  test-4\n");
 
     // 3 was accessed last in the read
     // So the order should be 2 3 4 1
@@ -826,19 +820,11 @@ fn test_ls_order_time() {
 
         // It seems to be dependent on the platform whether the access time is actually set
         if file3_access > file4_access {
-            if cfg!(not(windows)) {
             result.stdout_only("test-3\ntest-4\ntest-2\ntest-1\n");
-            } else {
-                result.stdout_only("test-3  test-4  test-2  test-1\n");
-            }
         } else {
             // Access time does not seem to be set on Windows and some other
             // systems so the order is 4 3 2 1
-            if cfg!(not(windows)) {
             result.stdout_only("test-4\ntest-3\ntest-2\ntest-1\n");
-            } else {
-                result.stdout_only("test-4  test-3  test-2  test-1\n");
-            }
         }
     }
 
@@ -991,6 +977,7 @@ fn test_ls_color() {
         .ucmd()
         .arg("--color")
         .arg("-w=15")
+        .arg("-C")
         .succeeds()
         .stdout_only(format!(
             "{}  test-color\nb  {}\n",
@@ -2009,11 +1996,7 @@ fn test_ls_path() {
     };
     scene.ucmd().arg(&abs_path).run().stdout_is(expected_stdout);
 
-    let expected_stdout = if cfg!(windows) {
+    let expected_stdout = format!("{}\n{}\n", path, file1);
-        format!("{}  {}\n", path, file1)
-    } else {
-        format!("{}\n{}\n", path, file1)
-    };
     scene
         .ucmd()
         .arg(file1)

tests/by-util/test_od.rs

@@ -46,6 +46,17 @@ fn test_file() {
         .succeeds()
         .no_stderr()
         .stdout_is(unindent(ALPHA_OUT));
+
+    // Ensure that default format matches `-t o2`, and that `-t` does not absorb file argument
+    new_ucmd!()
+        .arg("--endian=little")
+        .arg("-t")
+        .arg("o2")
+        .arg(file.as_os_str())
+        .succeeds()
+        .no_stderr()
+        .stdout_is(unindent(ALPHA_OUT));
+
     let _ = remove_file(file);
 }
 

tests/by-util/test_pr.rs

@@ -68,41 +68,36 @@ fn test_with_numbering_option_with_number_width() {
 fn test_with_long_header_option() {
     let test_file_path = "test_one_page.log";
     let expected_test_file_path = "test_one_page_header.log.expected";
+    let header = "new file";
+    for args in &[&["-h", header][..], &["--header=new file"][..]] {
         let mut scenario = new_ucmd!();
         let value = file_last_modified_time(&scenario, test_file_path);
-    let header = "new file";
         scenario
-        .args(&["--header=new file", test_file_path])
+            .args(args)
-        .succeeds()
+            .arg(test_file_path)
-        .stdout_is_templated_fixture(
-            expected_test_file_path,
-            &[("{last_modified_time}", &value), ("{header}", header)],
-        );
-
-    new_ucmd!()
-        .args(&["-h", header, test_file_path])
             .succeeds()
             .stdout_is_templated_fixture(
                 expected_test_file_path,
                 &[("{last_modified_time}", &value), ("{header}", header)],
             );
+    }
 }
 
 #[test]
 fn test_with_double_space_option() {
     let test_file_path = "test_one_page.log";
     let expected_test_file_path = "test_one_page_double_line.log.expected";
+    for &arg in &["-d", "--double-space"] {
         let mut scenario = new_ucmd!();
         let value = file_last_modified_time(&scenario, test_file_path);
         scenario
-        .args(&["-d", test_file_path])
+            .args(&[arg, test_file_path])
             .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
+            .stdout_is_templated_fixture(
-
+                expected_test_file_path,
-    new_ucmd!()
+                &[("{last_modified_time}", &value)],
-        .args(&["--double-space", test_file_path])
+            );
-        .succeeds()
+    }
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
 }
 
 #[test]
@@ -188,33 +183,28 @@ fn test_with_page_range() {
     let test_file_path = "test.log";
     let expected_test_file_path = "test_page_range_1.log.expected";
     let expected_test_file_path1 = "test_page_range_2.log.expected";
+    for &arg in &["--pages=15", "+15"] {
         let mut scenario = new_ucmd!();
         let value = file_last_modified_time(&scenario, test_file_path);
         scenario
-        .args(&["--pages=15", test_file_path])
+            .args(&[arg, test_file_path])
             .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
+            .stdout_is_templated_fixture(
-
+                expected_test_file_path,
-    new_ucmd!()
+                &[("{last_modified_time}", &value)],
-        .args(&["+15", test_file_path])
+            );
-        .succeeds()
+    }
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
+    for &arg in &["--pages=15:17", "+15:17"] {
-
+        let mut scenario = new_ucmd!();
-    new_ucmd!()
+        let value = file_last_modified_time(&scenario, test_file_path);
-        .args(&["--pages=15:17", test_file_path])
+        scenario
-        .succeeds()
+            .args(&[arg, test_file_path])
-        .stdout_is_templated_fixture(
-            expected_test_file_path1,
-            &[("{last_modified_time}", &value)],
-        );
-
-    new_ucmd!()
-        .args(&["+15:17", test_file_path])
             .succeeds()
             .stdout_is_templated_fixture(
                 expected_test_file_path1,
                 &[("{last_modified_time}", &value)],
             );
+    }
 }
 
 #[test]
@@ -232,19 +222,17 @@ fn test_with_no_header_trailer_option() {
 #[test]
 fn test_with_page_length_option() {
     let test_file_path = "test.log";
-    let expected_test_file_path = "test_page_length.log.expected";
+    for (arg, expected) in &[
-    let expected_test_file_path1 = "test_page_length1.log.expected";
+        ("100", "test_page_length.log.expected"),
+        ("5", "test_page_length1.log.expected"),
+    ] {
         let mut scenario = new_ucmd!();
         let value = file_last_modified_time(&scenario, test_file_path);
         scenario
-        .args(&["--pages=2:3", "-l", "100", "-n", test_file_path])
+            .args(&["--pages=2:3", "-l", arg, "-n", test_file_path])
             .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
+            .stdout_is_templated_fixture(expected, &[("{last_modified_time}", &value)]);
-
+    }
-    new_ucmd!()
-        .args(&["--pages=2:3", "-l", "5", "-n", test_file_path])
-        .succeeds()
-        .stdout_is_fixture(expected_test_file_path1);
 }
 
 #[test]
@@ -277,17 +265,17 @@ fn test_with_stdin() {
 fn test_with_column() {
     let test_file_path = "column.log";
     let expected_test_file_path = "column.log.expected";
+    for arg in &["-3", "--column=3"] {
         let mut scenario = new_ucmd!();
         let value = file_last_modified_time(&scenario, test_file_path);
         scenario
-        .args(&["--pages=3:5", "--column=3", "-n", test_file_path])
+            .args(&["--pages=3:5", arg, "-n", test_file_path])
             .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
+            .stdout_is_templated_fixture(
-
+                expected_test_file_path,
-    new_ucmd!()
+                &[("{last_modified_time}", &value)],
-        .args(&["--pages=3:5", "-3", "-n", test_file_path])
+            );
-        .succeeds()
+    }
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
 }
 
 #[test]
@@ -305,36 +293,17 @@ fn test_with_column_across_option() {
 #[test]
 fn test_with_column_across_option_and_column_separator() {
     let test_file_path = "column.log";
-    let expected_test_file_path = "column_across_sep.log.expected";
+    for (arg, expected) in &[
-    let expected_test_file_path1 = "column_across_sep1.log.expected";
+        ("-s|", "column_across_sep.log.expected"),
+        ("-Sdivide", "column_across_sep1.log.expected"),
+    ] {
         let mut scenario = new_ucmd!();
         let value = file_last_modified_time(&scenario, test_file_path);
         scenario
-        .args(&[
+            .args(&["--pages=3:5", "--column=3", arg, "-a", "-n", test_file_path])
-            "--pages=3:5",
-            "--column=3",
-            "-s|",
-            "-a",
-            "-n",
-            test_file_path,
-        ])
             .succeeds()
-        .stdout_is_templated_fixture(expected_test_file_path, &[("{last_modified_time}", &value)]);
+            .stdout_is_templated_fixture(expected, &[("{last_modified_time}", &value)]);
-
+    }
-    new_ucmd!()
-        .args(&[
-            "--pages=3:5",
-            "--column=3",
-            "-Sdivide",
-            "-a",
-            "-n",
-            test_file_path,
-        ])
-        .succeeds()
-        .stdout_is_templated_fixture(
-            expected_test_file_path1,
-            &[("{last_modified_time}", &value)],
-        );
 }
 
 #[test]

util/build-gnu.sh

@@ -96,7 +96,6 @@ sed -i 's|seq |/usr/bin/seq |' tests/misc/sort-discrim.sh
 
 # Add specific timeout to tests that currently hang to limit time spent waiting
 sed -i 's|seq \$|/usr/bin/timeout 0.1 seq \$|' tests/misc/seq-precision.sh tests/misc/seq-long-double.sh
-sed -i 's|cat |/usr/bin/timeout 0.1 cat |' tests/misc/cat-self.sh
 
 
 # Remove dup of /usr/bin/ when executed several times

util/compare_gnu_result.py

@@ -0,0 +1,32 @@
+#! /usr/bin/python
+
+"""
+Compare the current results to the last results gathered from the master branch to highlight
+if a PR is making the results better/worse
+"""
+
+import json
+import sys
+from os import environ
+
+NEW = json.load(open("gnu-result.json"))
+OLD = json.load(open("master-gnu-result.json"))
+
+# Extract the specific results from the dicts
+last = OLD[list(OLD.keys())[0]]
+current = NEW[list(NEW.keys())[0]]
+
+
+pass_d = int(current["pass"]) - int(last["pass"])
+fail_d = int(current["fail"]) - int(last["fail"])
+error_d = int(current["error"]) - int(last["error"])
+skip_d = int(current["skip"]) - int(last["skip"])
+
+# Get an annotation to highlight changes
+print(
+    f"::warning ::Changes from master: PASS {pass_d:+d} / FAIL {fail_d:+d} / ERROR {error_d:+d} / SKIP {skip_d:+d} "
+)
+
+# If results are worse fail the job to draw attention
+if pass_d < 0:
+    sys.exit(1)