Merge branch 'master' into issue2167

.gitignore

@@ -12,3 +12,6 @@ target/
 Cargo.lock
 lib*.a
 /docs/_build
+*.iml
+### macOS ###
+.DS_Store

Cargo.lock

@@ -618,7 +618,7 @@ checksum = "1d34cfa13a63ae058bfa601fe9e313bbdb3746427c1459185464ce0fcf62e1e8"
 dependencies = [
  "cfg-if 1.0.0",
  "libc",
- "redox_syscall 0.2.7",
+ "redox_syscall 0.2.8",
  "winapi 0.3.9",
 ]
 
@@ -1259,9 +1259,9 @@ checksum = "41cc0f7e4d5d4544e8861606a285bb08d3e70712ccc7d2b84d7c0ccfaf4b05ce"
 
 [[package]]
 name = "redox_syscall"
-version = "0.2.7"
+version = "0.2.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "85dd92e586f7355c633911e11f77f3d12f04b1b1bd76a198bd34ae3af8341ef2"
+checksum = "742739e41cd49414de871ea5e549afb7e2a3ac77b589bcbebe8c82fab37147fc"
 dependencies = [
  "bitflags",
 ]
@@ -1272,14 +1272,14 @@ version = "0.1.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "8440d8acb4fd3d277125b4bd01a6f38aee8d814b3b5fc09b3f2b825d37d3fe8f"
 dependencies = [
- "redox_syscall 0.2.7",
+ "redox_syscall 0.2.8",
 ]
 
 [[package]]
 name = "regex"
-version = "1.5.3"
+version = "1.5.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ce5f1ceb7f74abbce32601642fcf8e8508a8a8991e0621c7d750295b9095702b"
+checksum = "d07a8629359eb56f1e2fb1652bb04212c072a87ba68546a04065d525673ac461"
 dependencies = [
  "aho-corasick",
  "memchr 2.4.0",
@@ -1312,9 +1312,9 @@ dependencies = [
 
 [[package]]
 name = "retain_mut"
-version = "0.1.2"
+version = "0.1.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "53552c6c49e1e13f1a203ef0080ab3bbef0beb570a528993e83df057a9d9bba1"
+checksum = "e9c17925a9027d298a4603d286befe3f9dc0e8ed02523141914eb628798d6e5b"
 
 [[package]]
 name = "rust-ini"
@@ -1372,9 +1372,6 @@ name = "serde"
 version = "1.0.125"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "558dc50e1a5a5fa7112ca2ce4effcb321b0300c0d4ccf0776a9f60cd89031171"
-dependencies = [
- "serde_derive",
-]
 
 [[package]]
 name = "serde_cbor"
@@ -1453,9 +1450,6 @@ name = "smallvec"
 version = "1.6.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "fe0f37c9e8f3c5a4a66ad655a93c74daac4ad00c441533bf5c6e7990bb42604e"
-dependencies = [
- "serde",
-]
 
 [[package]]
 name = "strsim"
@@ -1483,9 +1477,9 @@ dependencies = [
 
 [[package]]
 name = "syn"
-version = "1.0.71"
+version = "1.0.72"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "ad184cc9470f9117b2ac6817bfe297307418819ba40552f9b3846f05c33d5373"
+checksum = "a1e8cdbefb79a9a5a65e0db8b47b723ee907b7c7f8496c76a1770b5c310bab82"
 dependencies = [
  "proc-macro2",
  "quote 1.0.9",
@@ -1543,7 +1537,7 @@ checksum = "077185e2eac69c3f8379a4298e1e07cd36beb962290d4a51199acf0fdc10607e"
 dependencies = [
  "libc",
  "numtoa",
- "redox_syscall 0.2.7",
+ "redox_syscall 0.2.8",
  "redox_termios",
 ]
 
@@ -1698,6 +1692,7 @@ dependencies = [
 name = "uu_basename"
 version = "0.0.6"
 dependencies = [
+ "clap",
  "uucore",
  "uucore_procs",
 ]
@@ -2073,6 +2068,7 @@ dependencies = [
 name = "uu_logname"
 version = "0.0.6"
 dependencies = [
+ "clap",
  "libc",
  "uucore",
  "uucore_procs",
@@ -2122,7 +2118,7 @@ dependencies = [
 name = "uu_mknod"
 version = "0.0.6"
 dependencies = [
- "getopts",
+ "clap",
  "libc",
  "uucore",
  "uucore_procs",
@@ -2396,8 +2392,6 @@ dependencies = [
  "rand 0.7.3",
  "rayon",
  "semver",
- "serde",
- "serde_json",
  "smallvec 1.6.1",
  "tempdir",
  "unicode-width",
@@ -2653,6 +2647,7 @@ dependencies = [
 name = "uu_who"
 version = "0.0.6"
 dependencies = [
+ "clap",
  "uucore",
  "uucore_procs",
 ]

src/uu/basename/Cargo.toml

@@ -15,6 +15,7 @@ edition = "2018"
 path = "src/basename.rs"
 
 [dependencies]
+clap = "2.33.2"
 uucore = { version=">=0.0.8", package="uucore", path="../../uucore" }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
 

src/uu/basename/src/basename.rs

@@ -10,83 +10,106 @@
 #[macro_use]
 extern crate uucore;
 
+use clap::{App, Arg};
 use std::path::{is_separator, PathBuf};
 use uucore::InvalidEncodingHandling;
 
-static NAME: &str = "basename";
+static VERSION: &str = env!("CARGO_PKG_VERSION");
-static SYNTAX: &str = "NAME [SUFFIX]";
 static SUMMARY: &str = "Print NAME with any leading directory components removed
- If specified, also remove a trailing SUFFIX";
+If specified, also remove a trailing SUFFIX";
-static LONG_HELP: &str = "";
+
+fn get_usage() -> String {
+    format!(
+        "{0} NAME [SUFFIX]
+    {0} OPTION... NAME...",
+        executable!()
+    )
+}
+
+pub mod options {
+    pub static MULTIPLE: &str = "multiple";
+    pub static NAME: &str = "name";
+    pub static SUFFIX: &str = "suffix";
+    pub static ZERO: &str = "zero";
+}
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
     let args = args
         .collect_str(InvalidEncodingHandling::ConvertLossy)
         .accept_any();
+    let usage = get_usage();
     //
     // Argument parsing
     //
-    let matches = app!(SYNTAX, SUMMARY, LONG_HELP)
+    let matches = App::new(executable!())
-        .optflag(
+        .version(VERSION)
-            "a",
+        .about(SUMMARY)
-            "multiple",
+        .usage(&usage[..])
-            "Support more than one argument. Treat every argument as a name.",
+        .arg(
+            Arg::with_name(options::MULTIPLE)
+                .short("a")
+                .long(options::MULTIPLE)
+                .help("support multiple arguments and treat each as a NAME"),
         )
-        .optopt(
+        .arg(Arg::with_name(options::NAME).multiple(true).hidden(true))
-            "s",
+        .arg(
-            "suffix",
+            Arg::with_name(options::SUFFIX)
-            "Remove a trailing suffix. This option implies the -a option.",
+                .short("s")
-            "SUFFIX",
+                .long(options::SUFFIX)
+                .value_name("SUFFIX")
+                .help("remove a trailing SUFFIX; implies -a"),
         )
-        .optflag(
+        .arg(
-            "z",
+            Arg::with_name(options::ZERO)
-            "zero",
+                .short("z")
-            "Output a zero byte (ASCII NUL) at the end of each line, rather than a newline.",
+                .long(options::ZERO)
+                .help("end each output line with NUL, not newline"),
         )
-        .parse(args);
+        .get_matches_from(args);
 
     // too few arguments
-    if matches.free.is_empty() {
+    if !matches.is_present(options::NAME) {
         crash!(
             1,
-            "{0}: {1}\nTry '{0} --help' for more information.",
+            "{1}\nTry '{0} --help' for more information.",
-            NAME,
+            executable!(),
             "missing operand"
         );
     }
-    let opt_s = matches.opt_present("s");
+
-    let opt_a = matches.opt_present("a");
+    let opt_suffix = matches.is_present(options::SUFFIX);
-    let opt_z = matches.opt_present("z");
+    let opt_multiple = matches.is_present(options::MULTIPLE);
-    let multiple_paths = opt_s || opt_a;
+    let opt_zero = matches.is_present(options::ZERO);
+    let multiple_paths = opt_suffix || opt_multiple;
     // too many arguments
-    if !multiple_paths && matches.free.len() > 2 {
+    if !multiple_paths && matches.occurrences_of(options::NAME) > 2 {
         crash!(
             1,
-            "{0}: extra operand '{1}'\nTry '{0} --help' for more information.",
+            "extra operand '{1}'\nTry '{0} --help' for more information.",
-            NAME,
+            executable!(),
-            matches.free[2]
+            matches.values_of(options::NAME).unwrap().nth(2).unwrap()
         );
     }
 
-    let suffix = if opt_s {
+    let suffix = if opt_suffix {
-        matches.opt_str("s").unwrap()
+        matches.value_of(options::SUFFIX).unwrap()
-    } else if !opt_a && matches.free.len() > 1 {
+    } else if !opt_multiple && matches.occurrences_of(options::NAME) > 1 {
-        matches.free[1].clone()
+        matches.values_of(options::NAME).unwrap().nth(1).unwrap()
     } else {
-        "".to_owned()
+        ""
     };
 
     //
     // Main Program Processing
     //
 
-    let paths = if multiple_paths {
+    let paths: Vec<_> = if multiple_paths {
-        &matches.free[..]
+        matches.values_of(options::NAME).unwrap().collect()
     } else {
-        &matches.free[0..1]
+        matches.values_of(options::NAME).unwrap().take(1).collect()
     };
 
-    let line_ending = if opt_z { "\0" } else { "\n" };
+    let line_ending = if opt_zero { "\0" } else { "\n" };
     for path in paths {
         print!("{}{}", basename(&path, &suffix), line_ending);
     }

src/uu/chmod/src/chmod.rs

@@ -15,6 +15,7 @@ use std::fs;
 use std::os::unix::fs::{MetadataExt, PermissionsExt};
 use std::path::Path;
 use uucore::fs::display_permissions_unix;
+use uucore::libc::mode_t;
 #[cfg(not(windows))]
 use uucore::mode;
 use uucore::InvalidEncodingHandling;
@@ -306,7 +307,7 @@ impl Chmoder {
                     "mode of '{}' retained as {:04o} ({})",
                     file.display(),
                     fperm,
-                    display_permissions_unix(fperm),
+                    display_permissions_unix(fperm as mode_t, false),
                 );
             }
             Ok(())
@@ -319,9 +320,9 @@ impl Chmoder {
                     "failed to change mode of file '{}' from {:o} ({}) to {:o} ({})",
                     file.display(),
                     fperm,
-                    display_permissions_unix(fperm),
+                    display_permissions_unix(fperm as mode_t, false),
                     mode,
-                    display_permissions_unix(mode)
+                    display_permissions_unix(mode as mode_t, false)
                 );
             }
             Err(1)
@@ -331,9 +332,9 @@ impl Chmoder {
                     "mode of '{}' changed from {:o} ({}) to {:o} ({})",
                     file.display(),
                     fperm,
-                    display_permissions_unix(fperm),
+                    display_permissions_unix(fperm as mode_t, false),
                     mode,
-                    display_permissions_unix(mode)
+                    display_permissions_unix(mode as mode_t, false)
                 );
             }
             Ok(())

src/uu/df/src/df.rs

@@ -916,6 +916,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
             "Use%",
         ]
     });
+    if cfg!(target_os = "macos") && !opt.show_inode_instead {
+        header.insert(header.len() - 1, "Capacity");
+    }
     header.push("Mounted on");
 
     for (idx, title) in header.iter().enumerate() {
@@ -970,6 +973,11 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
                 "{0: >12} ",
                 human_readable(free_size, opt.human_readable_base)
             );
+            if cfg!(target_os = "macos") {
+                let used = fs.usage.blocks - fs.usage.bfree;
+                let blocks = used + fs.usage.bavail;
+                print!("{0: >12} ", use_size(used, blocks));
+            }
             print!("{0: >5} ", use_size(free_size, total_size));
         }
         print!("{0: <16}", fs.mountinfo.mount_dir);

src/uu/fmt/src/linebreak.rs

@@ -296,7 +296,7 @@ fn find_kp_breakpoints<'a, T: Iterator<Item = &'a WordInfo<'a>>>(
                         (0, 0.0)
                     } else {
                         compute_demerits(
-                            (args.opts.goal - tlen) as isize,
+                            args.opts.goal as isize - tlen as isize,
                             stretch,
                             w.word_nchars as isize,
                             active.prev_rat,

src/uu/logname/Cargo.toml

@@ -16,6 +16,7 @@ path = "src/logname.rs"
 
 [dependencies]
 libc = "0.2.42"
+clap = "2.33"
 uucore = { version=">=0.0.8", package="uucore", path="../../uucore" }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
 

src/uu/logname/src/logname.rs

@@ -15,6 +15,8 @@ extern crate uucore;
 use std::ffi::CStr;
 use uucore::InvalidEncodingHandling;
 
+use clap::App;
+
 extern "C" {
     // POSIX requires using getlogin (or equivalent code)
     pub fn getlogin() -> *const libc::c_char;
@@ -31,15 +33,24 @@ fn get_userlogin() -> Option<String> {
     }
 }
 
-static SYNTAX: &str = "";
 static SUMMARY: &str = "Print user's login name";
-static LONG_HELP: &str = "";
+static VERSION: &str = env!("CARGO_PKG_VERSION");
+
+fn get_usage() -> String {
+    String::from(executable!())
+}
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
-    app!(SYNTAX, SUMMARY, LONG_HELP).parse(
+    let args = args
-        args.collect_str(InvalidEncodingHandling::ConvertLossy)
+        .collect_str(InvalidEncodingHandling::Ignore)
-            .accept_any(),
+        .accept_any();
-    );
+
+    let usage = get_usage();
+    let _ = App::new(executable!())
+        .version(VERSION)
+        .about(SUMMARY)
+        .usage(&usage[..])
+        .get_matches_from(args);
 
     match get_userlogin() {
         Some(userlogin) => println!("{}", userlogin),

src/uu/ls/src/ls.rs

@@ -1179,31 +1179,32 @@ impl PathData {
 }
 
 fn list(locs: Vec<String>, config: Config) -> i32 {
-    let number_of_locs = locs.len();
-
     let mut files = Vec::<PathData>::new();
     let mut dirs = Vec::<PathData>::new();
     let mut has_failed = false;
 
     let mut out = BufWriter::new(stdout());
 
-    for loc in locs {
+    for loc in &locs {
         let p = PathBuf::from(&loc);
         if !p.exists() {
             show_error!("'{}': {}", &loc, "No such file or directory");
-            // We found an error, the return code of ls should not be 0
+            /*
-            // And no need to continue the execution
+            We found an error, the return code of ls should not be 0
+            And no need to continue the execution
+            */
             has_failed = true;
             continue;
         }
 
         let path_data = PathData::new(p, None, None, &config, true);
 
-        let show_dir_contents = if let Some(ft) = path_data.file_type() {
+        let show_dir_contents = match path_data.file_type() {
-            !config.directory && ft.is_dir()
+            Some(ft) => !config.directory && ft.is_dir(),
-        } else {
+            None => {
                 has_failed = true;
                 false
+            }
         };
 
         if show_dir_contents {
@@ -1217,7 +1218,7 @@ fn list(locs: Vec<String>, config: Config) -> i32 {
 
     sort_entries(&mut dirs, &config);
     for dir in dirs {
-        if number_of_locs > 1 {
+        if locs.len() > 1 {
             let _ = writeln!(out, "\n{}:", dir.p_buf.display());
         }
         enter_directory(&dir, &config, &mut out);
@@ -1331,7 +1332,7 @@ fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize) {
     if let Some(md) = entry.md() {
         (
             display_symlink_count(&md).len(),
-            display_file_size(&md, config).len(),
+            display_size(md.len(), config).len(),
         )
     } else {
         (0, 0)
@@ -1344,14 +1345,22 @@ fn pad_left(string: String, count: usize) -> String {
 
 fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter<Stdout>) {
     if config.format == Format::Long {
-        let (mut max_links, mut max_size) = (1, 1);
+        let (mut max_links, mut max_width) = (1, 1);
+        let mut total_size = 0;
+
         for item in items {
-            let (links, size) = display_dir_entry_size(item, config);
+            let (links, width) = display_dir_entry_size(item, config);
             max_links = links.max(max_links);
-            max_size = size.max(max_size);
+            max_width = width.max(max_width);
+            total_size += item.md().map_or(0, |md| get_block_size(md, config));
         }
+
+        if total_size > 0 {
+            let _ = writeln!(out, "total {}", display_size(total_size, config));
+        }
+
         for item in items {
-            display_item_long(item, max_links, max_size, config, out);
+            display_item_long(item, max_links, max_width, config, out);
         }
     } else {
         let names = items.iter().filter_map(|i| display_file_name(&i, config));
@@ -1396,6 +1405,29 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter<Stdout
     }
 }
 
+fn get_block_size(md: &Metadata, config: &Config) -> u64 {
+    /* GNU ls will display sizes in terms of block size
+       md.len() will differ from this value when the file has some holes
+    */
+    #[cfg(unix)]
+    {
+        // hard-coded for now - enabling setting this remains a TODO
+        let ls_block_size = 1024;
+        return match config.size_format {
+            SizeFormat::Binary => md.blocks() * 512,
+            SizeFormat::Decimal => md.blocks() * 512,
+            SizeFormat::Bytes => md.blocks() * 512 / ls_block_size,
+        };
+    }
+
+    #[cfg(not(unix))]
+    {
+        let _ = config;
+        // no way to get block size for windows, fall-back to file size
+        md.len()
+    }
+}
+
 fn display_grid(
     names: impl Iterator<Item = Cell>,
     width: u16,
@@ -1448,9 +1480,8 @@ fn display_item_long(
 
     let _ = write!(
         out,
-        "{}{} {}",
+        "{} {}",
-        display_file_type(md.file_type()),
+        display_permissions(&md, true),
-        display_permissions(&md),
         pad_left(display_symlink_count(&md), max_links),
     );
 
@@ -1471,7 +1502,7 @@ fn display_item_long(
     let _ = writeln!(
         out,
         " {} {} {}",
-        pad_left(display_file_size(&md, config), max_size),
+        pad_left(display_size(md.len(), config), max_size),
         display_date(&md, config),
         // unwrap is fine because it fails when metadata is not available
         // but we already know that it is because it's checked at the
@@ -1626,23 +1657,13 @@ fn format_prefixed(prefixed: NumberPrefix<f64>) -> String {
     }
 }
 
-fn display_file_size(metadata: &Metadata, config: &Config) -> String {
+fn display_size(len: u64, config: &Config) -> String {
     // NOTE: The human-readable behaviour deviates from the GNU ls.
     // The GNU ls uses binary prefixes by default.
     match config.size_format {
-        SizeFormat::Binary => format_prefixed(NumberPrefix::binary(metadata.len() as f64)),
+        SizeFormat::Binary => format_prefixed(NumberPrefix::binary(len as f64)),
-        SizeFormat::Decimal => format_prefixed(NumberPrefix::decimal(metadata.len() as f64)),
+        SizeFormat::Decimal => format_prefixed(NumberPrefix::decimal(len as f64)),
-        SizeFormat::Bytes => metadata.len().to_string(),
+        SizeFormat::Bytes => len.to_string(),
-    }
-}
-
-fn display_file_type(file_type: FileType) -> char {
-    if file_type.is_dir() {
-        'd'
-    } else if file_type.is_symlink() {
-        'l'
-    } else {
-        '-'
     }
 }
 

src/uu/mknod/Cargo.toml

@@ -16,7 +16,7 @@ name = "uu_mknod"
 path = "src/mknod.rs"
 
 [dependencies]
-getopts = "0.2.18"
+clap = "2.33"
 libc = "^0.2.42"
 uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["mode"] }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }

src/uu/mknod/src/mknod.rs

@@ -5,21 +5,41 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
-// spell-checker:ignore (ToDO) parsemode makedev sysmacros makenod newmode perror IFBLK IFCHR IFIFO
+// spell-checker:ignore (ToDO) parsemode makedev sysmacros perror IFBLK IFCHR IFIFO
 
 #[macro_use]
 extern crate uucore;
 
+use std::ffi::CString;
+
+use clap::{App, Arg, ArgMatches};
 use libc::{dev_t, mode_t};
 use libc::{S_IFBLK, S_IFCHR, S_IFIFO, S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR};
 
-use getopts::Options;
-
-use std::ffi::CString;
 use uucore::InvalidEncodingHandling;
 
 static NAME: &str = "mknod";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
+static ABOUT: &str = "Create the special file NAME of the given TYPE.";
+static USAGE: &str = "mknod [OPTION]... NAME TYPE [MAJOR MINOR]";
+static LONG_HELP: &str = "Mandatory arguments to long options are mandatory for short options too.
+-m, --mode=MODE    set file permission bits to MODE, not a=rw - umask
+--help     display this help and exit
+--version  output version information and exit
+
+Both MAJOR and MINOR must be specified when TYPE is b, c, or u, and they
+must be omitted when TYPE is p.  If MAJOR or MINOR begins with 0x or 0X,
+it is interpreted as hexadecimal; otherwise, if it begins with 0, as octal;
+otherwise, as decimal.  TYPE may be:
+
+b      create a block (buffered) special file
+c, u   create a character (unbuffered) special file
+p      create a FIFO
+
+NOTE: your shell may have its own version of mknod, which usually supersedes
+the version described here.  Please refer to your shell's documentation
+for details about the options it supports.
+";
 
 const MODE_RW_UGO: mode_t = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
 
@@ -30,13 +50,35 @@ fn makedev(maj: u64, min: u64) -> dev_t {
 }
 
 #[cfg(windows)]
-fn _makenod(path: CString, mode: mode_t, dev: dev_t) -> i32 {
+fn _makenod(file_name: &str, mode: mode_t, dev: dev_t) -> i32 {
     panic!("Unsupported for windows platform")
 }
 
 #[cfg(unix)]
-fn _makenod(path: CString, mode: mode_t, dev: dev_t) -> i32 {
+fn _makenod(file_name: &str, mode: mode_t, dev: dev_t) -> i32 {
-    unsafe { libc::mknod(path.as_ptr(), mode, dev) }
+    let c_str = CString::new(file_name).expect("Failed to convert to CString");
+
+    // the user supplied a mode
+    let set_umask = mode & MODE_RW_UGO != MODE_RW_UGO;
+
+    unsafe {
+        // store prev umask
+        let last_umask = if set_umask { libc::umask(0) } else { 0 };
+
+        let errno = libc::mknod(c_str.as_ptr(), mode, dev);
+
+        // set umask back to original value
+        if set_umask {
+            libc::umask(last_umask);
+        }
+
+        if errno == -1 {
+            let c_str = CString::new(NAME).expect("Failed to convert to CString");
+            // shows the error from the mknod syscall
+            libc::perror(c_str.as_ptr());
+        }
+        errno
+    }
 }
 
 #[allow(clippy::cognitive_complexity)]
@@ -44,156 +86,136 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     let args = args
         .collect_str(InvalidEncodingHandling::Ignore)
         .accept_any();
-
-    let mut opts = Options::new();
-
     // Linux-specific options, not implemented
     // opts.optflag("Z", "", "set the SELinux security context to default type");
     // opts.optopt("", "context", "like -Z, or if CTX is specified then set the SELinux or SMACK security context to CTX");
-    opts.optopt(
-        "m",
-        "mode",
-        "set file permission bits to MODE, not a=rw - umask",
-        "MODE",
-    );
 
-    opts.optflag("", "help", "display this help and exit");
+    let matches = App::new(executable!())
-    opts.optflag("", "version", "output version information and exit");
+        .version(VERSION)
+        .usage(USAGE)
+        .after_help(LONG_HELP)
+        .about(ABOUT)
+        .arg(
+            Arg::with_name("mode")
+                .short("m")
+                .long("mode")
+                .value_name("MODE")
+                .help("set file permission bits to MODE, not a=rw - umask"),
+        )
+        .arg(
+            Arg::with_name("name")
+                .value_name("NAME")
+                .help("name of the new file")
+                .required(true)
+                .index(1),
+        )
+        .arg(
+            Arg::with_name("type")
+                .value_name("TYPE")
+                .help("type of the new file (b, c, u or p)")
+                .required(true)
+                .validator(valid_type)
+                .index(2),
+        )
+        .arg(
+            Arg::with_name("major")
+                .value_name("MAJOR")
+                .help("major file type")
+                .validator(valid_u64)
+                .index(3),
+        )
+        .arg(
+            Arg::with_name("minor")
+                .value_name("MINOR")
+                .help("minor file type")
+                .validator(valid_u64)
+                .index(4),
+        )
+        .get_matches_from(args);
 
-    let matches = match opts.parse(&args[1..]) {
+    let mode = match get_mode(&matches) {
-        Ok(m) => m,
+        Ok(mode) => mode,
-        Err(f) => crash!(1, "{}\nTry '{} --help' for more information.", f, NAME),
+        Err(err) => {
+            show_info!("{}", err);
+            return 1;
+        }
     };
 
-    if matches.opt_present("help") {
+    let file_name = matches.value_of("name").expect("Missing argument 'NAME'");
-        println!(
-            "Usage: {0} [OPTION]... NAME TYPE [MAJOR MINOR]
-
-Mandatory arguments to long options are mandatory for short options too.
-  -m, --mode=MODE    set file permission bits to MODE, not a=rw - umask
-      --help     display this help and exit
-      --version  output version information and exit
-
-Both MAJOR and MINOR must be specified when TYPE is b, c, or u, and they
-must be omitted when TYPE is p.  If MAJOR or MINOR begins with 0x or 0X,
-it is interpreted as hexadecimal; otherwise, if it begins with 0, as octal;
-otherwise, as decimal.  TYPE may be:
-
-  b      create a block (buffered) special file
-  c, u   create a character (unbuffered) special file
-  p      create a FIFO
-
-NOTE: your shell may have its own version of mknod, which usually supersedes
-the version described here.  Please refer to your shell's documentation
-for details about the options it supports.",
-            NAME
-        );
-        return 0;
-    }
-
-    if matches.opt_present("version") {
-        println!("{} {}", NAME, VERSION);
-        return 0;
-    }
-
-    let mut last_umask: mode_t = 0;
-    let mut newmode: mode_t = MODE_RW_UGO;
-    if matches.opt_present("mode") {
-        match uucore::mode::parse_mode(matches.opt_str("mode")) {
-            Ok(parsed) => {
-                if parsed > 0o777 {
-                    show_info!("mode must specify only file permission bits");
-                    return 1;
-                }
-                newmode = parsed;
-            }
-            Err(e) => {
-                show_info!("{}", e);
-                return 1;
-            }
-        }
-        unsafe {
-            last_umask = libc::umask(0);
-        }
-    }
-
-    let mut ret = 0i32;
-    match matches.free.len() {
-        0 => show_usage_error!("missing operand"),
-        1 => show_usage_error!("missing operand after ‘{}’", matches.free[0]),
-        _ => {
-            let args = &matches.free;
-            let c_str = CString::new(args[0].as_str()).expect("Failed to convert to CString");
 
     // Only check the first character, to allow mnemonic usage like
     // 'mknod /dev/rst0 character 18 0'.
-            let ch = args[1]
+    let ch = matches
+        .value_of("type")
+        .expect("Missing argument 'TYPE'")
         .chars()
         .next()
         .expect("Failed to get the first char");
 
     if ch == 'p' {
-                if args.len() > 2 {
+        if matches.is_present("major") || matches.is_present("minor") {
-                    show_info!("{}: extra operand ‘{}’", NAME, args[2]);
-                    if args.len() == 4 {
             eprintln!("Fifos do not have major and minor device numbers.");
-                    }
             eprintln!("Try '{} --help' for more information.", NAME);
-                    return 1;
+            1
-                }
-
-                ret = _makenod(c_str, S_IFIFO | newmode, 0);
         } else {
-                if args.len() < 4 {
+            _makenod(file_name, S_IFIFO | mode, 0)
-                    show_info!("missing operand after ‘{}’", args[args.len() - 1]);
+        }
-                    if args.len() == 2 {
+    } else {
+        match (matches.value_of("major"), matches.value_of("minor")) {
+            (None, None) | (_, None) | (None, _) => {
                 eprintln!("Special files require major and minor device numbers.");
-                    }
                 eprintln!("Try '{} --help' for more information.", NAME);
-                    return 1;
+                1
-                } else if args.len() > 4 {
-                    show_usage_error!("extra operand ‘{}’", args[4]);
-                    return 1;
-                } else if !"bcu".contains(ch) {
-                    show_usage_error!("invalid device type ‘{}’", args[1]);
-                    return 1;
             }
+            (Some(major), Some(minor)) => {
+                let major = major.parse::<u64>().expect("validated by clap");
+                let minor = minor.parse::<u64>().expect("validated by clap");
 
-                let maj = args[2].parse::<u64>();
+                let dev = makedev(major, minor);
-                let min = args[3].parse::<u64>();
-                if maj.is_err() {
-                    show_info!("invalid major device number ‘{}’", args[2]);
-                    return 1;
-                } else if min.is_err() {
-                    show_info!("invalid minor device number ‘{}’", args[3]);
-                    return 1;
-                }
-
-                let (maj, min) = (maj.unwrap(), min.unwrap());
-                let dev = makedev(maj, min);
                 if ch == 'b' {
                     // block special file
-                    ret = _makenod(c_str, S_IFBLK | newmode, dev);
+                    _makenod(file_name, S_IFBLK | mode, dev)
-                } else {
+                } else if ch == 'c' || ch == 'u' {
                     // char special file
-                    ret = _makenod(c_str, S_IFCHR | newmode, dev);
+                    _makenod(file_name, S_IFCHR | mode, dev)
+                } else {
+                    unreachable!("{} was validated to be only b, c or u", ch);
                 }
             }
         }
     }
-
+}
-    if last_umask != 0 {
+
-        unsafe {
+fn get_mode(matches: &ArgMatches) -> Result<mode_t, String> {
-            libc::umask(last_umask);
+    match matches.value_of("mode") {
-        }
+        None => Ok(MODE_RW_UGO),
-    }
+        Some(str_mode) => uucore::mode::parse_mode(str_mode)
-    if ret == -1 {
+            .map_err(|e| format!("invalid mode ({})", e))
-        let c_str = CString::new(format!("{}: {}", NAME, matches.free[0]).as_str())
+            .and_then(|mode| {
-            .expect("Failed to convert to CString");
+                if mode > 0o777 {
-        unsafe {
+                    Err("mode must specify only file permission bits".to_string())
-            libc::perror(c_str.as_ptr());
+                } else {
-        }
+                    Ok(mode)
-    }
+                }
-
+            }),
-    ret
+    }
+}
+
+fn valid_type(tpe: String) -> Result<(), String> {
+    // Only check the first character, to allow mnemonic usage like
+    // 'mknod /dev/rst0 character 18 0'.
+    tpe.chars()
+        .next()
+        .ok_or_else(|| "missing device type".to_string())
+        .and_then(|first_char| {
+            if vec!['b', 'c', 'u', 'p'].contains(&first_char) {
+                Ok(())
+            } else {
+                Err(format!("invalid device type ‘{}’", tpe))
+            }
+        })
+}
+
+fn valid_u64(num: String) -> Result<(), String> {
+    num.parse::<u64>().map(|_| ()).map_err(|_| num)
 }

src/uu/mknod/src/parsemode.rs

@@ -0,0 +1,54 @@
+// spell-checker:ignore (ToDO) fperm
+
+use libc::{mode_t, S_IRGRP, S_IROTH, S_IRUSR, S_IWGRP, S_IWOTH, S_IWUSR};
+
+use uucore::mode;
+
+pub const MODE_RW_UGO: mode_t = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
+
+pub fn parse_mode(mode: &str) -> Result<mode_t, String> {
+    let arr: &[char] = &['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'];
+    let result = if mode.contains(arr) {
+        mode::parse_numeric(MODE_RW_UGO as u32, mode)
+    } else {
+        mode::parse_symbolic(MODE_RW_UGO as u32, mode, true)
+    };
+    result.map(|mode| mode as mode_t)
+}
+
+#[cfg(test)]
+mod test {
+    /// Test if the program is running under WSL
+    // ref: <https://github.com/microsoft/WSL/issues/4555> @@ <https://archive.is/dP0bz>
+    // ToDO: test on WSL2 which likely doesn't need special handling; plan change to `is_wsl_1()` if WSL2 is less needy
+    pub fn is_wsl() -> bool {
+        #[cfg(target_os = "linux")]
+        {
+            if let Ok(b) = std::fs::read("/proc/sys/kernel/osrelease") {
+                if let Ok(s) = std::str::from_utf8(&b) {
+                    let a = s.to_ascii_lowercase();
+                    return a.contains("microsoft") || a.contains("wsl");
+                }
+            }
+        }
+        false
+    }
+
+    #[test]
+    fn symbolic_modes() {
+        assert_eq!(super::parse_mode("u+x").unwrap(), 0o766);
+        assert_eq!(
+            super::parse_mode("+x").unwrap(),
+            if !is_wsl() { 0o777 } else { 0o776 }
+        );
+        assert_eq!(super::parse_mode("a-w").unwrap(), 0o444);
+        assert_eq!(super::parse_mode("g-r").unwrap(), 0o626);
+    }
+
+    #[test]
+    fn numeric_modes() {
+        assert_eq!(super::parse_mode("644").unwrap(), 0o644);
+        assert_eq!(super::parse_mode("+100").unwrap(), 0o766);
+        assert_eq!(super::parse_mode("-4").unwrap(), 0o662);
+    }
+}

src/uu/sort/BENCHMARKING.md

@@ -69,6 +69,14 @@ Run `cargo build --release` before benchmarking after you make a change!
 
 -   Benchmark numeric sorting with hyperfine: `hyperfine "target/release/coreutils sort shuffled_numbers_si.txt -h -o output.txt"`.
 
+## External sorting
+
+Try running commands with the `-S` option set to an amount of memory to be used, such as `1M`. Additionally, you could try sorting
+huge files (ideally multiple Gigabytes) with `-S`. Creating such a large file can be achieved by running `cat shuffled_wordlist.txt | sort -R >> shuffled_wordlist.txt`
+multiple times (this will add the contents of `shuffled_wordlist.txt` to itself).
+Example: Run `hyperfine './target/release/coreutils sort shuffled_wordlist.txt -S 1M' 'sort shuffled_wordlist.txt -S 1M'`
+`
+
 ## Stdout and stdin performance
 
 Try to run the above benchmarks by piping the input through stdin (standard input) and redirect the

src/uu/sort/Cargo.toml

@@ -15,15 +15,13 @@ edition = "2018"
 path = "src/sort.rs"
 
 [dependencies]
-serde_json = { version = "1.0.64", default-features = false, features = ["alloc"] }
-serde = { version = "1.0", features = ["derive"] }
 rayon = "1.5"
 rand = "0.7"
 clap = "2.33"
 fnv = "1.0.7"
 itertools = "0.10.0"
 semver = "0.9.0"
-smallvec = { version="1.6.1", features=["serde"] } 
+smallvec = "1.6.1"
 unicode-width = "0.1.8"
 uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["fs"] }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }

src/uu/sort/src/external_sort/mod.rs

@@ -1,295 +1,93 @@
-use std::clone::Clone;
+use std::fs::OpenOptions;
-use std::cmp::Ordering::Less;
+use std::io::{BufWriter, Write};
-use std::collections::VecDeque;
+use std::path::Path;
-use std::error::Error;
-use std::fs::{File, OpenOptions};
-use std::io::SeekFrom::Start;
-use std::io::{BufRead, BufReader, BufWriter, Seek, Write};
-use std::marker::PhantomData;
-use std::path::PathBuf;
 
-use serde::de::DeserializeOwned;
-use serde::Serialize;
-use serde_json;
 use tempdir::TempDir;
 
+use crate::{file_to_lines_iter, FileMerger};
+
 use super::{GlobalSettings, Line};
 
-/// Trait for types that can be used by
-/// [ExternalSorter](struct.ExternalSorter.html). Must be sortable, cloneable,
-/// serializeable, and able to report on it's size
-pub trait ExternallySortable: Clone + Serialize + DeserializeOwned {
-    /// Get the size, in bytes, of this object (used to constrain the buffer
-    /// used in the external sort).
-    fn get_size(&self) -> u64;
-}
-
 /// Iterator that provides sorted `T`s
-pub struct ExtSortedIterator<Line> {
+pub struct ExtSortedIterator<'a> {
-    buffers: Vec<VecDeque<Line>>,
+    file_merger: FileMerger<'a>,
-    chunk_offsets: Vec<u64>,
+    // Keep tmp_dir around, it is deleted when dropped.
-    max_per_chunk: u64,
+    _tmp_dir: TempDir,
-    chunks: u64,
-    tmp_dir: TempDir,
-    settings: GlobalSettings,
-    failed: bool,
 }
 
-impl Iterator for ExtSortedIterator<Line>
+impl<'a> Iterator for ExtSortedIterator<'a> {
-where
+    type Item = Line;
-    Line: ExternallySortable,
-{
-    type Item = Result<Line, Box<dyn Error>>;
-
-    /// # Errors
-    ///
-    /// This method can fail due to issues reading intermediate sorted chunks
-    /// from disk, or due to serde deserialization issues
     fn next(&mut self) -> Option<Self::Item> {
-        if self.failed {
+        self.file_merger.next()
-            return None;
-        }
-        // fill up any empty buffers
-        let mut empty = true;
-        for chunk_num in 0..self.chunks {
-            if self.buffers[chunk_num as usize].is_empty() {
-                let mut f = match File::open(self.tmp_dir.path().join(chunk_num.to_string())) {
-                    Ok(f) => f,
-                    Err(e) => {
-                        self.failed = true;
-                        return Some(Err(Box::new(e)));
-                    }
-                };
-                match f.seek(Start(self.chunk_offsets[chunk_num as usize])) {
-                    Ok(_) => (),
-                    Err(e) => {
-                        self.failed = true;
-                        return Some(Err(Box::new(e)));
-                    }
-                }
-                let bytes_read =
-                    match fill_buff(&mut self.buffers[chunk_num as usize], f, self.max_per_chunk) {
-                        Ok(bytes_read) => bytes_read,
-                        Err(e) => {
-                            self.failed = true;
-                            return Some(Err(e));
-                        }
-                    };
-                self.chunk_offsets[chunk_num as usize] += bytes_read;
-                if !self.buffers[chunk_num as usize].is_empty() {
-                    empty = false;
-                }
-            } else {
-                empty = false;
-            }
-        }
-        if empty {
-            return None;
-        }
-
-        // find the next record to write
-        // check is_empty() before unwrap()ing
-        let mut idx = 0;
-        for chunk_num in 0..self.chunks as usize {
-            if !self.buffers[chunk_num].is_empty() {
-                if self.buffers[idx].is_empty()
-                    || (super::compare_by)(
-                        self.buffers[chunk_num].front().unwrap(),
-                        self.buffers[idx].front().unwrap(),
-                        &self.settings,
-                    ) == Less
-                {
-                    idx = chunk_num;
-                }
-            }
-        }
-
-        // unwrap due to checks above
-        let r = self.buffers[idx].pop_front().unwrap();
-        Some(Ok(r))
     }
 }
 
-/// Perform an external sort on an unsorted stream of incoming data
+/// Sort (based on `compare`) the `T`s provided by `unsorted` and return an
-pub struct ExternalSorter<Line>
+/// iterator
-where
+///
-    Line: ExternallySortable,
+/// # Panics
-{
+///
-    tmp_dir: Option<PathBuf>,
+/// This method can panic due to issues writing intermediate sorted chunks
-    buffer_bytes: u64,
+/// to disk.
-    phantom: PhantomData<Line>,
+pub fn ext_sort(
-    settings: GlobalSettings,
+    unsorted: impl Iterator<Item = Line>,
-}
+    settings: &GlobalSettings,
+) -> ExtSortedIterator {
+    let tmp_dir = crash_if_err!(1, TempDir::new_in(&settings.tmp_dir, "uutils_sort"));
 
-impl ExternalSorter<Line>
-where
-    Line: ExternallySortable,
-{
-    /// Create a new `ExternalSorter` with a specified memory buffer and
-    /// temporary directory
-    pub fn new(
-        buffer_bytes: u64,
-        tmp_dir: Option<PathBuf>,
-        settings: GlobalSettings,
-    ) -> ExternalSorter<Line> {
-        ExternalSorter {
-            buffer_bytes,
-            tmp_dir,
-            phantom: PhantomData,
-            settings,
-        }
-    }
-
-    /// Sort (based on `compare`) the `T`s provided by `unsorted` and return an
-    /// iterator
-    ///
-    /// # Errors
-    ///
-    /// This method can fail due to issues writing intermediate sorted chunks
-    /// to disk, or due to serde serialization issues
-    pub fn sort_by<I>(
-        &self,
-        unsorted: I,
-        settings: GlobalSettings,
-    ) -> Result<ExtSortedIterator<Line>, Box<dyn Error>>
-    where
-        I: Iterator<Item = Line>,
-    {
-        let tmp_dir = match self.tmp_dir {
-            Some(ref p) => TempDir::new_in(p, "uutils_sort")?,
-            None => TempDir::new("uutils_sort")?,
-        };
-        // creating the thing we need to return first due to the face that we need to
-        // borrow tmp_dir and move it out
-        let mut iter = ExtSortedIterator {
-            buffers: Vec::new(),
-            chunk_offsets: Vec::new(),
-            max_per_chunk: 0,
-            chunks: 0,
-            tmp_dir,
-            settings,
-            failed: false,
-        };
-
-        {
     let mut total_read = 0;
     let mut chunk = Vec::new();
-            // Initial buffer is specified by user
+
-            let mut adjusted_buffer_size = self.buffer_bytes;
+    let mut chunks_read = 0;
-            let (iter_size, _) = unsorted.size_hint();
+    let mut file_merger = FileMerger::new(settings);
 
     // make the initial chunks on disk
     for seq in unsorted {
-                let seq_size = seq.get_size();
+        let seq_size = seq.estimate_size();
         total_read += seq_size;
 
-                // GNU minimum is 16 * (sizeof struct + 2), but GNU uses about
-                // 1/10 the memory that we do.  And GNU even says in the code it may
-                // not work on small buffer sizes.
-                //
-                // The following seems to work pretty well, and has about the same max
-                // RSS as lower minimum values.
-                //
-                let minimum_buffer_size: u64 = iter_size as u64 * seq_size / 8;
-
-                adjusted_buffer_size =
-                    // Grow buffer size for a struct/Line larger than buffer
-                    if adjusted_buffer_size < seq_size {
-                        seq_size
-                    } else if adjusted_buffer_size < minimum_buffer_size {
-                        minimum_buffer_size
-                    } else {
-                        adjusted_buffer_size
-                    };
         chunk.push(seq);
 
-                if total_read >= adjusted_buffer_size {
+        if total_read >= settings.buffer_size && chunk.len() >= 2 {
-                    super::sort_by(&mut chunk, &self.settings);
+            super::sort_by(&mut chunk, &settings);
-                    self.write_chunk(
+
-                        &iter.tmp_dir.path().join(iter.chunks.to_string()),
+            let file_path = tmp_dir.path().join(chunks_read.to_string());
-                        &mut chunk,
+            write_chunk(settings, &file_path, &mut chunk);
-                    )?;
             chunk.clear();
             total_read = 0;
-                    iter.chunks += 1;
+            chunks_read += 1;
+
+            file_merger.push_file(Box::new(file_to_lines_iter(file_path, settings).unwrap()))
         }
     }
     // write the last chunk
-            if chunk.len() > 0 {
+    if !chunk.is_empty() {
-                super::sort_by(&mut chunk, &self.settings);
+        super::sort_by(&mut chunk, &settings);
-                self.write_chunk(
+
-                    &iter.tmp_dir.path().join(iter.chunks.to_string()),
+        let file_path = tmp_dir.path().join(chunks_read.to_string());
+        write_chunk(
+            settings,
+            &tmp_dir.path().join(chunks_read.to_string()),
             &mut chunk,
-                )?;
+        );
-                iter.chunks += 1;
-            }
 
-            // initialize buffers for each chunk
+        file_merger.push_file(Box::new(file_to_lines_iter(file_path, settings).unwrap()));
-            //
-            // Having a right sized buffer for each chunk for smallish values seems silly to me?
-            //
-            // We will have to have the entire iter in memory sometime right?
-            // Set minimum to the size of the writer buffer, ~8K
-            //
-            const MINIMUM_READBACK_BUFFER: u64 = 8200;
-            let right_sized_buffer = adjusted_buffer_size
-                .checked_div(iter.chunks)
-                .unwrap_or(adjusted_buffer_size);
-            iter.max_per_chunk = if right_sized_buffer > MINIMUM_READBACK_BUFFER {
-                right_sized_buffer
-            } else {
-                MINIMUM_READBACK_BUFFER
-            };
-            iter.buffers = vec![VecDeque::new(); iter.chunks as usize];
-            iter.chunk_offsets = vec![0 as u64; iter.chunks as usize];
-            for chunk_num in 0..iter.chunks {
-                let offset = fill_buff(
-                    &mut iter.buffers[chunk_num as usize],
-                    File::open(iter.tmp_dir.path().join(chunk_num.to_string()))?,
-                    iter.max_per_chunk,
-                )?;
-                iter.chunk_offsets[chunk_num as usize] = offset;
     }
+    ExtSortedIterator {
+        file_merger,
+        _tmp_dir: tmp_dir,
     }
+}
 
-        Ok(iter)
+fn write_chunk(settings: &GlobalSettings, file: &Path, chunk: &mut Vec<Line>) {
-    }
+    let new_file = crash_if_err!(1, OpenOptions::new().create(true).append(true).open(file));
-
+    let mut buf_write = BufWriter::new(new_file);
-    fn write_chunk(&self, file: &PathBuf, chunk: &mut Vec<Line>) -> Result<(), Box<dyn Error>> {
-        let new_file = OpenOptions::new().create(true).append(true).open(file)?;
-        let mut buf_write = Box::new(BufWriter::new(new_file)) as Box<dyn Write>;
     for s in chunk {
-            let mut serialized = serde_json::to_string(&s).expect("JSON write error: ");
+        crash_if_err!(1, buf_write.write_all(s.line.as_bytes()));
-            serialized.push_str("\n");
+        crash_if_err!(
-            buf_write.write(serialized.as_bytes())?;
+            1,
-        }
+            buf_write.write_all(if settings.zero_terminated { "\0" } else { "\n" }.as_bytes(),)
-        buf_write.flush()?;
+        );
-
-        Ok(())
     }
-}
+    crash_if_err!(1, buf_write.flush());
-
-fn fill_buff<Line>(
-    vec: &mut VecDeque<Line>,
-    file: File,
-    max_bytes: u64,
-) -> Result<u64, Box<dyn Error>>
-where
-    Line: ExternallySortable,
-{
-    let mut total_read = 0;
-    let mut bytes_read = 0;
-    for line in BufReader::new(file).lines() {
-        let line_s = line?;
-        bytes_read += line_s.len() + 1;
-        // This is where the bad stuff happens usually
-        let deserialized: Line = serde_json::from_str(&line_s).expect("JSON read error: ");
-        total_read += deserialized.get_size();
-        vec.push_back(deserialized);
-        if total_read > max_bytes {
-            break;
-        }
-    }
-
-    Ok(bytes_read as u64)
 }

src/uu/sort/src/numeric_str_cmp.rs

@@ -14,21 +14,20 @@
 //! More specifically, exponent can be understood so that the original number is in (1..10)*10^exponent.
 //! From that follows the constraints of this algorithm: It is able to compare numbers in ±(1*10^[i64::MIN]..10*10^[i64::MAX]).
 
-use serde::{Deserialize, Serialize};
 use std::{cmp::Ordering, ops::Range};
 
-#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize, Clone)]
+#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Clone)]
 enum Sign {
     Negative,
     Positive,
 }
 
-#[derive(Debug, PartialEq, Serialize, Deserialize, Clone)]
+#[derive(Debug, PartialEq, Clone)]
 pub struct NumInfo {
     exponent: i64,
     sign: Sign,
 }
-#[derive(Debug, PartialEq, Serialize, Deserialize, Clone)]
+#[derive(Debug, PartialEq, Clone)]
 pub struct NumInfoParseSettings {
     pub accept_si_units: bool,
     pub thousands_separator: Option<char>,

src/uu/sort/src/sort.rs

@@ -21,7 +21,7 @@ mod numeric_str_cmp;
 
 use clap::{App, Arg};
 use custom_str_cmp::custom_str_cmp;
-use external_sort::{ExternalSorter, ExternallySortable};
+use external_sort::ext_sort;
 use fnv::FnvHasher;
 use itertools::Itertools;
 use numeric_str_cmp::{numeric_str_cmp, NumInfo, NumInfoParseSettings};
@@ -29,14 +29,14 @@ use rand::distributions::Alphanumeric;
 use rand::{thread_rng, Rng};
 use rayon::prelude::*;
 use semver::Version;
-use serde::{Deserialize, Serialize};
 use smallvec::SmallVec;
 use std::cmp::Ordering;
 use std::collections::BinaryHeap;
 use std::env;
+use std::ffi::OsStr;
 use std::fs::File;
 use std::hash::{Hash, Hasher};
-use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Lines, Read, Write};
+use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Write};
 use std::mem::replace;
 use std::ops::Range;
 use std::path::Path;
@@ -106,7 +106,7 @@ enum SortMode {
     Default,
 }
 #[derive(Clone)]
-struct GlobalSettings {
+pub struct GlobalSettings {
     mode: SortMode,
     debug: bool,
     ignore_blanks: bool,
@@ -206,7 +206,7 @@ impl From<&GlobalSettings> for KeySettings {
     }
 }
 
-#[derive(Debug, Serialize, Deserialize, Clone)]
+#[derive(Debug, Clone)]
 /// Represents the string selected by a FieldSelector.
 struct SelectionRange {
     range: Range<usize>,
@@ -228,7 +228,7 @@ impl SelectionRange {
     }
 }
 
-#[derive(Serialize, Deserialize, Clone)]
+#[derive(Clone)]
 enum NumCache {
     AsF64(GeneralF64ParseResult),
     WithInfo(NumInfo),
@@ -249,7 +249,8 @@ impl NumCache {
         }
     }
 }
-#[derive(Serialize, Deserialize, Clone)]
+
+#[derive(Clone)]
 struct Selection {
     range: SelectionRange,
     num_cache: NumCache,
@@ -264,22 +265,21 @@ impl Selection {
 
 type Field = Range<usize>;
 
-#[derive(Serialize, Deserialize, Clone)]
+#[derive(Clone)]
-struct Line {
+pub struct Line {
     line: String,
     // The common case is not to specify fields. Let's make this fast.
     selections: SmallVec<[Selection; 1]>,
 }
 
-impl ExternallySortable for Line {
-    fn get_size(&self) -> u64 {
-        // Currently 96 bytes, but that could change, so we get that size here
-        std::mem::size_of::<Line>() as u64
-    }
-}
-
 impl Line {
-    fn new(line: String, settings: &GlobalSettings) -> Self {
+    pub fn estimate_size(&self) -> usize {
+        self.line.capacity()
+            + self.selections.capacity() * std::mem::size_of::<Selection>()
+            + std::mem::size_of::<Self>()
+    }
+
+    pub fn new(line: String, settings: &GlobalSettings) -> Self {
         let fields = if settings
             .selectors
             .iter()
@@ -291,7 +291,7 @@ impl Line {
             None
         };
 
-        let selections = settings
+        let selections: SmallVec<[Selection; 1]> = settings
             .selectors
             .iter()
             .map(|selector| {
@@ -683,7 +683,7 @@ impl FieldSelector {
 }
 
 struct MergeableFile<'a> {
-    lines: Lines<BufReader<Box<dyn Read>>>,
+    lines: Box<dyn Iterator<Item = Line> + 'a>,
     current_line: Line,
     settings: &'a GlobalSettings,
 }
@@ -723,11 +723,11 @@ impl<'a> FileMerger<'a> {
             settings,
         }
     }
-    fn push_file(&mut self, mut lines: Lines<BufReader<Box<dyn Read>>>) {
+    fn push_file(&mut self, mut lines: Box<dyn Iterator<Item = Line> + 'a>) {
-        if let Some(Ok(next_line)) = lines.next() {
+        if let Some(next_line) = lines.next() {
             let mergeable_file = MergeableFile {
                 lines,
-                current_line: Line::new(next_line, &self.settings),
+                current_line: next_line,
                 settings: &self.settings,
             };
             self.heap.push(mergeable_file);
@@ -741,11 +741,8 @@ impl<'a> Iterator for FileMerger<'a> {
         match self.heap.pop() {
             Some(mut current) => {
                 match current.lines.next() {
-                    Some(Ok(next_line)) => {
+                    Some(next_line) => {
-                        let ret = replace(
+                        let ret = replace(&mut current.current_line, next_line);
-                            &mut current.current_line,
-                            Line::new(next_line, &self.settings),
-                        );
                         self.heap.push(current);
                         Some(ret)
                     }
@@ -1113,82 +1110,98 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     exec(files, settings)
 }
 
-fn exec(files: Vec<String>, settings: GlobalSettings) -> i32 {
+fn file_to_lines_iter(
-    let mut lines = Vec::new();
+    file: impl AsRef<OsStr>,
-    let mut file_merger = FileMerger::new(&settings);
+    settings: &'_ GlobalSettings,
-
+) -> Option<impl Iterator<Item = Line> + '_> {
-    for path in &files {
+    let (reader, _) = match open(file) {
-        let (reader, _) = match open(path) {
         Some(x) => x,
-            None => continue,
+        None => return None,
     };
 
     let buf_reader = BufReader::new(reader);
 
-        if settings.merge {
+    Some(
-            file_merger.push_file(buf_reader.lines());
+        buf_reader
-        } else if settings.zero_terminated {
+            .split(if settings.zero_terminated {
-            for line in buf_reader.split(b'\0').flatten() {
+                b'\0'
-                lines.push(Line::new(
+            } else {
-                    std::str::from_utf8(&line)
+                b'\n'
-                        .expect("Could not parse string from zero terminated input.")
+            })
-                        .to_string(),
+            .map(move |line| {
+                Line::new(
+                    crash_if_err!(1, String::from_utf8(crash_if_err!(1, line))),
+                    settings,
+                )
+            }),
+    )
+}
+
+fn output_sorted_lines(iter: impl Iterator<Item = Line>, settings: &GlobalSettings) {
+    if settings.unique {
+        print_sorted(
+            iter.dedup_by(|a, b| compare_by(a, b, &settings) == Ordering::Equal),
             &settings,
-                ));
+        );
-            }
     } else {
-            for line in buf_reader.lines() {
+        print_sorted(iter, &settings);
-                if let Ok(n) = line {
+    }
-                    lines.push(Line::new(n, &settings));
+}
+
+fn exec(files: Vec<String>, settings: GlobalSettings) -> i32 {
+    if settings.merge {
+        let mut file_merger = FileMerger::new(&settings);
+        for lines in files
+            .iter()
+            .filter_map(|file| file_to_lines_iter(file, &settings))
+        {
+            file_merger.push_file(Box::new(lines));
+        }
+        output_sorted_lines(file_merger, &settings);
     } else {
-                    break;
+        let lines = files
-                }
+            .iter()
-            }
+            .filter_map(|file| file_to_lines_iter(file, &settings))
-        }
+            .flatten();
-    }
 
         if settings.check {
-        return exec_check_file(&lines, &settings);
+            return exec_check_file(lines, &settings);
         }
 
         // Only use ext_sorter when we need to.
         // Probably faster that we don't create
         // an owned value each run
         if settings.ext_sort {
-        lines = ext_sort_by(lines, settings.clone());
+            let sorted_lines = ext_sort(lines, &settings);
+            output_sorted_lines(sorted_lines, &settings);
         } else {
-        sort_by(&mut lines, &settings);
+            let mut lines = vec![];
+
+            // This is duplicated from fn file_to_lines_iter, but using that function directly results in a performance regression.
+            for (file, _) in files.iter().map(open).flatten() {
+                let buf_reader = BufReader::new(file);
+                for line in buf_reader.split(if settings.zero_terminated {
+                    b'\0'
+                } else {
+                    b'\n'
+                }) {
+                    let string = crash_if_err!(1, String::from_utf8(crash_if_err!(1, line)));
+                    lines.push(Line::new(string, &settings));
+                }
             }
 
-    if settings.merge {
+            sort_by(&mut lines, &settings);
-        if settings.unique {
+            output_sorted_lines(lines.into_iter(), &settings);
-            print_sorted(
-                file_merger.dedup_by(|a, b| compare_by(a, b, &settings) == Ordering::Equal),
-                &settings,
-            )
-        } else {
-            print_sorted(file_merger, &settings)
         }
-    } else if settings.unique {
-        print_sorted(
-            lines
-                .into_iter()
-                .dedup_by(|a, b| compare_by(a, b, &settings) == Ordering::Equal),
-            &settings,
-        )
-    } else {
-        print_sorted(lines.into_iter(), &settings)
     }
 
     0
 }
 
-fn exec_check_file(unwrapped_lines: &[Line], settings: &GlobalSettings) -> i32 {
+fn exec_check_file(unwrapped_lines: impl Iterator<Item = Line>, settings: &GlobalSettings) -> i32 {
     // errors yields the line before each disorder,
     // plus the last line (quirk of .coalesce())
-    let mut errors =
+    let mut errors = unwrapped_lines
-        unwrapped_lines
-            .iter()
         .enumerate()
         .coalesce(|(last_i, last_line), (i, line)| {
             if compare_by(&last_line, &line, &settings) == Ordering::Greater {
@@ -1215,20 +1228,6 @@ fn exec_check_file(unwrapped_lines: &[Line], settings: &GlobalSettings) -> i32 {
     }
 }
 
-fn ext_sort_by(unsorted: Vec<Line>, settings: GlobalSettings) -> Vec<Line> {
-    let external_sorter = ExternalSorter::new(
-        settings.buffer_size as u64,
-        Some(settings.tmp_dir.clone()),
-        settings.clone(),
-    );
-    let iter = external_sorter
-        .sort_by(unsorted.into_iter(), settings)
-        .unwrap()
-        .map(|x| x.unwrap())
-        .collect::<Vec<Line>>();
-    iter
-}
-
 fn sort_by(unsorted: &mut Vec<Line>, settings: &GlobalSettings) {
     if settings.stable || settings.unique {
         unsorted.par_sort_by(|a, b| compare_by(a, b, &settings))
@@ -1332,7 +1331,7 @@ fn get_leading_gen(input: &str) -> Range<usize> {
     leading_whitespace_len..input.len()
 }
 
-#[derive(Serialize, Deserialize, Copy, Clone, PartialEq, PartialOrd)]
+#[derive(Copy, Clone, PartialEq, PartialOrd)]
 enum GeneralF64ParseResult {
     Invalid,
     NaN,
@@ -1503,7 +1502,8 @@ fn print_sorted<T: Iterator<Item = Line>>(iter: T, settings: &GlobalSettings) {
 }
 
 // from cat.rs
-fn open(path: &str) -> Option<(Box<dyn Read>, bool)> {
+fn open(path: impl AsRef<OsStr>) -> Option<(Box<dyn Read>, bool)> {
+    let path = path.as_ref();
     if path == "-" {
         let stdin = stdin();
         return Some((Box::new(stdin) as Box<dyn Read>, is_stdin_interactive()));
@@ -1512,7 +1512,7 @@ fn open(path: &str) -> Option<(Box<dyn Read>, bool)> {
     match File::open(Path::new(path)) {
         Ok(f) => Some((Box::new(f) as Box<dyn Read>, false)),
         Err(e) => {
-            show_error!("{0}: {1}", path, e.to_string());
+            show_error!("{0:?}: {1}", path, e.to_string());
             None
         }
     }

src/uu/split/src/split.rs

@@ -13,11 +13,11 @@ extern crate uucore;
 mod platform;
 
 use clap::{App, Arg};
-use std::char;
 use std::env;
 use std::fs::File;
-use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Read, Write};
+use std::io::{stdin, BufRead, BufReader, BufWriter, Read, Write};
 use std::path::Path;
+use std::{char, fs::remove_file};
 
 static NAME: &str = "split";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
@@ -213,107 +213,145 @@ struct Settings {
     verbose: bool,
 }
 
-struct SplitControl {
-    current_line: String,   // Don't touch
-    request_new_file: bool, // Splitter implementation requests new file
-}
-
 trait Splitter {
-    // Consume the current_line and return the consumed string
+    // Consume as much as possible from `reader` so as to saturate `writer`.
-    fn consume(&mut self, _: &mut SplitControl) -> String;
+    // Equivalent to finishing one of the part files. Returns the number of
+    // bytes that have been moved.
+    fn consume(
+        &mut self,
+        reader: &mut BufReader<Box<dyn Read>>,
+        writer: &mut BufWriter<Box<dyn Write>>,
+    ) -> u128;
 }
 
 struct LineSplitter {
-    saved_lines_to_write: usize,
+    lines_per_split: usize,
-    lines_to_write: usize,
 }
 
 impl LineSplitter {
     fn new(settings: &Settings) -> LineSplitter {
-        let n = match settings.strategy_param.parse() {
-            Ok(a) => a,
-            Err(e) => crash!(1, "invalid number of lines: {}", e),
-        };
         LineSplitter {
-            saved_lines_to_write: n,
+            lines_per_split: settings
-            lines_to_write: n,
+                .strategy_param
+                .parse()
+                .unwrap_or_else(|e| crash!(1, "invalid number of lines: {}", e)),
         }
     }
 }
 
 impl Splitter for LineSplitter {
-    fn consume(&mut self, control: &mut SplitControl) -> String {
+    fn consume(
-        self.lines_to_write -= 1;
+        &mut self,
-        if self.lines_to_write == 0 {
+        reader: &mut BufReader<Box<dyn Read>>,
-            self.lines_to_write = self.saved_lines_to_write;
+        writer: &mut BufWriter<Box<dyn Write>>,
-            control.request_new_file = true;
+    ) -> u128 {
+        let mut bytes_consumed = 0u128;
+        let mut buffer = String::with_capacity(1024);
+        for _ in 0..self.lines_per_split {
+            let bytes_read = reader
+                .read_line(&mut buffer)
+                .unwrap_or_else(|_| crash!(1, "error reading bytes from input file"));
+            // If we ever read 0 bytes then we know we've hit EOF.
+            if bytes_read == 0 {
+                return bytes_consumed;
             }
-        control.current_line.clone()
+
+            writer
+                .write_all(buffer.as_bytes())
+                .unwrap_or_else(|_| crash!(1, "error writing bytes to output file"));
+            // Empty out the String buffer since `read_line` appends instead of
+            // replaces.
+            buffer.clear();
+
+            bytes_consumed += bytes_read as u128;
+        }
+
+        bytes_consumed
     }
 }
 
 struct ByteSplitter {
-    saved_bytes_to_write: usize,
+    bytes_per_split: u128,
-    bytes_to_write: usize,
-    break_on_line_end: bool,
-    require_whole_line: bool,
 }
 
 impl ByteSplitter {
     fn new(settings: &Settings) -> ByteSplitter {
-        let mut strategy_param: Vec<char> = settings.strategy_param.chars().collect();
+        // These multipliers are the same as supported by GNU coreutils.
-        let suffix = strategy_param.pop().unwrap();
+        let modifiers: Vec<(&str, u128)> = vec![
-        let multiplier = match suffix {
+            ("K", 1024u128),
-            '0'..='9' => 1usize,
+            ("M", 1024 * 1024),
-            'b' => 512usize,
+            ("G", 1024 * 1024 * 1024),
-            'k' => 1024usize,
+            ("T", 1024 * 1024 * 1024 * 1024),
-            'm' => 1024usize * 1024usize,
+            ("P", 1024 * 1024 * 1024 * 1024 * 1024),
-            _ => crash!(1, "invalid number of bytes"),
+            ("E", 1024 * 1024 * 1024 * 1024 * 1024 * 1024),
-        };
+            ("Z", 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024),
-        let n = if suffix.is_alphabetic() {
+            ("Y", 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024 * 1024),
-            match strategy_param
+            ("KB", 1000),
+            ("MB", 1000 * 1000),
+            ("GB", 1000 * 1000 * 1000),
+            ("TB", 1000 * 1000 * 1000 * 1000),
+            ("PB", 1000 * 1000 * 1000 * 1000 * 1000),
+            ("EB", 1000 * 1000 * 1000 * 1000 * 1000 * 1000),
+            ("ZB", 1000 * 1000 * 1000 * 1000 * 1000 * 1000 * 1000),
+            ("YB", 1000 * 1000 * 1000 * 1000 * 1000 * 1000 * 1000 * 1000),
+        ];
+
+        // This sequential find is acceptable since none of the modifiers are
+        // suffixes of any other modifiers, a la Huffman codes.
+        let (suffix, multiplier) = modifiers
             .iter()
-                .cloned()
+            .find(|(suffix, _)| settings.strategy_param.ends_with(suffix))
-                .collect::<String>()
+            .unwrap_or(&("", 1));
-                .parse::<usize>()
+
-            {
+        // Try to parse the actual numeral.
-                Ok(a) => a,
+        let n = &settings.strategy_param[0..(settings.strategy_param.len() - suffix.len())]
-                Err(e) => crash!(1, "invalid number of bytes: {}", e),
+            .parse::<u128>()
-            }
+            .unwrap_or_else(|e| crash!(1, "invalid number of bytes: {}", e));
-        } else {
+
-            match settings.strategy_param.parse::<usize>() {
-                Ok(a) => a,
-                Err(e) => crash!(1, "invalid number of bytes: {}", e),
-            }
-        };
         ByteSplitter {
-            saved_bytes_to_write: n * multiplier,
+            bytes_per_split: n * multiplier,
-            bytes_to_write: n * multiplier,
-            break_on_line_end: settings.strategy == "b",
-            require_whole_line: false,
         }
     }
 }
 
 impl Splitter for ByteSplitter {
-    fn consume(&mut self, control: &mut SplitControl) -> String {
+    fn consume(
-        let line = control.current_line.clone();
+        &mut self,
-        let n = std::cmp::min(line.chars().count(), self.bytes_to_write);
+        reader: &mut BufReader<Box<dyn Read>>,
-        if self.require_whole_line && n < line.chars().count() {
+        writer: &mut BufWriter<Box<dyn Write>>,
-            self.bytes_to_write = self.saved_bytes_to_write;
+    ) -> u128 {
-            control.request_new_file = true;
+        // We buffer reads and writes. We proceed until `bytes_consumed` is
-            self.require_whole_line = false;
+        // equal to `self.bytes_per_split` or we reach EOF.
-            return "".to_owned();
+        let mut bytes_consumed = 0u128;
+        const BUFFER_SIZE: usize = 1024;
+        let mut buffer = [0u8; BUFFER_SIZE];
+        while bytes_consumed < self.bytes_per_split {
+            // Don't overshoot `self.bytes_per_split`! Note: Using std::cmp::min
+            // doesn't really work since we have to get types to match which
+            // can't be done in a way that keeps all conversions safe.
+            let bytes_desired = if (BUFFER_SIZE as u128) <= self.bytes_per_split - bytes_consumed {
+                BUFFER_SIZE
+            } else {
+                // This is a safe conversion since the difference must be less
+                // than BUFFER_SIZE in this branch.
+                (self.bytes_per_split - bytes_consumed) as usize
+            };
+            let bytes_read = reader
+                .read(&mut buffer[0..bytes_desired])
+                .unwrap_or_else(|_| crash!(1, "error reading bytes from input file"));
+            // If we ever read 0 bytes then we know we've hit EOF.
+            if bytes_read == 0 {
+                return bytes_consumed;
             }
-        self.bytes_to_write -= n;
+
-        if n == 0 {
+            writer
-            self.bytes_to_write = self.saved_bytes_to_write;
+                .write_all(&buffer[0..bytes_read])
-            control.request_new_file = true;
+                .unwrap_or_else(|_| crash!(1, "error writing bytes to output file"));
+
+            bytes_consumed += bytes_read as u128;
         }
-        if self.break_on_line_end && n == line.chars().count() {
+
-            self.require_whole_line = self.break_on_line_end;
+        bytes_consumed
-        }
-        line[..n].to_owned()
     }
 }
 
@@ -353,14 +391,13 @@ fn split(settings: &Settings) -> i32 {
     let mut reader = BufReader::new(if settings.input == "-" {
         Box::new(stdin()) as Box<dyn Read>
     } else {
-        let r = match File::open(Path::new(&settings.input)) {
+        let r = File::open(Path::new(&settings.input)).unwrap_or_else(|_| {
-            Ok(a) => a,
+            crash!(
-            Err(_) => crash!(
                 1,
                 "cannot open '{}' for reading: No such file or directory",
                 settings.input
-            ),
+            )
-        };
+        });
         Box::new(r) as Box<dyn Read>
     });
 
@@ -370,21 +407,9 @@ fn split(settings: &Settings) -> i32 {
         a => crash!(1, "strategy {} not supported", a),
     };
 
-    let mut control = SplitControl {
-        current_line: "".to_owned(), // Request new line
-        request_new_file: true,      // Request new file
-    };
-
-    let mut writer = BufWriter::new(Box::new(stdout()) as Box<dyn Write>);
     let mut fileno = 0;
     loop {
-        if control.current_line.chars().count() == 0 {
+        // Get a new part file set up, and construct `writer` for it.
-            match reader.read_line(&mut control.current_line) {
-                Ok(0) | Err(_) => break,
-                _ => {}
-            }
-        }
-        if control.request_new_file {
         let mut filename = settings.prefix.clone();
         filename.push_str(
             if settings.numeric_suffix {
@@ -395,23 +420,26 @@ fn split(settings: &Settings) -> i32 {
             .as_ref(),
         );
         filename.push_str(settings.additional_suffix.as_ref());
+        let mut writer = platform::instantiate_current_writer(&settings.filter, filename.as_str());
+
+        let bytes_consumed = splitter.consume(&mut reader, &mut writer);
+        writer
+            .flush()
+            .unwrap_or_else(|e| crash!(1, "error flushing to output file: {}", e));
+
+        // If we didn't write anything we should clean up the empty file, and
+        // break from the loop.
+        if bytes_consumed == 0 {
+            // The output file is only ever created if --filter isn't used.
+            // Complicated, I know...
+            if settings.filter.is_none() {
+                remove_file(filename)
+                    .unwrap_or_else(|e| crash!(1, "error removing empty file: {}", e));
+            }
+            break;
+        }
 
-            crash_if_err!(1, writer.flush());
         fileno += 1;
-            writer = platform::instantiate_current_writer(&settings.filter, filename.as_str());
-            control.request_new_file = false;
-            if settings.verbose {
-                println!("creating file '{}'", filename);
-            }
-        }
-
-        let consumed = splitter.consume(&mut control);
-        crash_if_err!(1, writer.write_all(consumed.as_bytes()));
-
-        let advance = consumed.chars().count();
-        let clone = control.current_line.clone();
-        let sl = clone;
-        control.current_line = sl[advance..sl.chars().count()].to_owned();
     }
     0
 }

src/uu/stat/Cargo.toml

@@ -18,7 +18,7 @@ path = "src/stat.rs"
 clap = "2.33"
 time = "0.1.40"
 libc = "0.2"
-uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["entries", "libc"] }
+uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["entries", "libc", "fs"] }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
 
 [[bin]]

src/uu/stat/src/fsext.rs

@@ -47,13 +47,6 @@ impl BirthTime for Metadata {
     }
 }
 
-#[macro_export]
-macro_rules! has {
-    ($mode:expr, $perm:expr) => {
-        $mode & $perm != 0
-    };
-}
-
 pub fn pretty_time(sec: i64, nsec: i64) -> String {
     // sec == seconds since UNIX_EPOCH
     // nsec == nanoseconds since (UNIX_EPOCH + sec)
@@ -87,65 +80,6 @@ pub fn pretty_filetype<'a>(mode: mode_t, size: u64) -> &'a str {
     }
 }
 
-pub fn pretty_access(mode: mode_t) -> String {
-    let mut result = String::with_capacity(10);
-    result.push(match mode & S_IFMT {
-        S_IFDIR => 'd',
-        S_IFCHR => 'c',
-        S_IFBLK => 'b',
-        S_IFREG => '-',
-        S_IFIFO => 'p',
-        S_IFLNK => 'l',
-        S_IFSOCK => 's',
-        // TODO: Other file types
-        _ => '?',
-    });
-
-    result.push(if has!(mode, S_IRUSR) { 'r' } else { '-' });
-    result.push(if has!(mode, S_IWUSR) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISUID as mode_t) {
-        if has!(mode, S_IXUSR) {
-            's'
-        } else {
-            'S'
-        }
-    } else if has!(mode, S_IXUSR) {
-        'x'
-    } else {
-        '-'
-    });
-
-    result.push(if has!(mode, S_IRGRP) { 'r' } else { '-' });
-    result.push(if has!(mode, S_IWGRP) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISGID as mode_t) {
-        if has!(mode, S_IXGRP) {
-            's'
-        } else {
-            'S'
-        }
-    } else if has!(mode, S_IXGRP) {
-        'x'
-    } else {
-        '-'
-    });
-
-    result.push(if has!(mode, S_IROTH) { 'r' } else { '-' });
-    result.push(if has!(mode, S_IWOTH) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISVTX as mode_t) {
-        if has!(mode, S_IXOTH) {
-            't'
-        } else {
-            'T'
-        }
-    } else if has!(mode, S_IXOTH) {
-        'x'
-    } else {
-        '-'
-    });
-
-    result
-}
-
 use std::borrow::Cow;
 use std::convert::{AsRef, From};
 use std::ffi::CString;

src/uu/stat/src/stat.rs

@@ -7,13 +7,13 @@
 
 // spell-checker:ignore (ToDO) mtab fsext showfs otype fmtstr prec ftype blocksize nlink rdev fnodes fsid namelen blksize inodes fstype iosize statfs gnulib NBLOCKSIZE
 
-#[macro_use]
 mod fsext;
 pub use crate::fsext::*;
 
 #[macro_use]
 extern crate uucore;
 use uucore::entries;
+use uucore::fs::display_permissions;
 
 use clap::{App, Arg, ArgMatches};
 use std::borrow::Cow;
@@ -568,7 +568,7 @@ impl Stater {
                                     }
                                     // access rights in human readable form
                                     'A' => {
-                                        arg = pretty_access(meta.mode() as mode_t);
+                                        arg = display_permissions(&meta, true);
                                         otype = OutputType::Str;
                                     }
                                     // number of blocks allocated (see %B)

src/uu/wc/src/countable.rs

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

src/uu/wc/src/wc.rs

@@ -11,19 +11,19 @@
 extern crate uucore;
 
 mod count_bytes;
+mod countable;
+mod wordcount;
 use count_bytes::count_bytes_fast;
+use countable::WordCountable;
+use wordcount::{TitledWordCount, WordCount};
 
 use clap::{App, Arg, ArgMatches};
 use thiserror::Error;
 
 use std::cmp::max;
 use std::fs::File;
-use std::io::{self, BufRead, BufReader, Read, StdinLock, Write};
+use std::io::{self, Write};
-use std::ops::{Add, AddAssign};
-#[cfg(unix)]
-use std::os::unix::io::AsRawFd;
 use std::path::Path;
-use std::str::from_utf8;
 
 #[derive(Error, Debug)]
 pub enum WcError {
@@ -82,77 +82,6 @@ impl Settings {
     }
 }
 
-#[cfg(unix)]
-trait WordCountable: AsRawFd + Read {
-    type Buffered: BufRead;
-    fn get_buffered(self) -> Self::Buffered;
-}
-#[cfg(not(unix))]
-trait WordCountable: Read {
-    type Buffered: BufRead;
-    fn get_buffered(self) -> Self::Buffered;
-}
-
-impl WordCountable for StdinLock<'_> {
-    type Buffered = Self;
-
-    fn get_buffered(self) -> Self::Buffered {
-        self
-    }
-}
-impl WordCountable for File {
-    type Buffered = BufReader<Self>;
-
-    fn get_buffered(self) -> Self::Buffered {
-        BufReader::new(self)
-    }
-}
-
-#[derive(Debug, Default, Copy, Clone)]
-struct WordCount {
-    bytes: usize,
-    chars: usize,
-    lines: usize,
-    words: usize,
-    max_line_length: usize,
-}
-
-impl Add for WordCount {
-    type Output = Self;
-
-    fn add(self, other: Self) -> Self {
-        Self {
-            bytes: self.bytes + other.bytes,
-            chars: self.chars + other.chars,
-            lines: self.lines + other.lines,
-            words: self.words + other.words,
-            max_line_length: max(self.max_line_length, other.max_line_length),
-        }
-    }
-}
-
-impl AddAssign for WordCount {
-    fn add_assign(&mut self, other: Self) {
-        *self = *self + other
-    }
-}
-
-impl WordCount {
-    fn with_title(self, title: &str) -> TitledWordCount {
-        TitledWordCount { title, count: self }
-    }
-}
-
-/// This struct supplements the actual word count with a title that is displayed
-/// to the user at the end of the program.
-/// The reason we don't simply include title in the `WordCount` struct is that
-/// it would result in unneccesary copying of `String`.
-#[derive(Debug, Default, Clone)]
-struct TitledWordCount<'a> {
-    title: &'a str,
-    count: WordCount,
-}
-
 static ABOUT: &str = "Display newline, word, and byte counts for each FILE, and a total line if
 more than one FILE is specified.";
 static VERSION: &str = env!("CARGO_PKG_VERSION");
@@ -233,18 +162,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     }
 }
 
-const CR: u8 = b'\r';
-const LF: u8 = b'\n';
-const SPACE: u8 = b' ';
-const TAB: u8 = b'\t';
-const SYN: u8 = 0x16_u8;
-const FF: u8 = 0x0C_u8;
-
-#[inline(always)]
-fn is_word_separator(byte: u8) -> bool {
-    byte == SPACE || byte == TAB || byte == CR || byte == SYN || byte == FF
-}
-
 fn word_count_from_reader<T: WordCountable>(
     mut reader: T,
     settings: &Settings,
@@ -265,69 +182,20 @@ fn word_count_from_reader<T: WordCountable>(
     // we do not need to decode the byte stream if we're only counting bytes/newlines
     let decode_chars = settings.show_chars || settings.show_words || settings.show_max_line_length;
 
-    let mut line_count: usize = 0;
+    // Sum the WordCount for each line. Show a warning for each line
-    let mut word_count: usize = 0;
+    // that results in an IO error when trying to read it.
-    let mut byte_count: usize = 0;
+    let total = reader
-    let mut char_count: usize = 0;
+        .lines()
-    let mut longest_line_length: usize = 0;
+        .filter_map(|res| match res {
-    let mut raw_line = Vec::new();
+            Ok(line) => Some(line),
-    let mut ends_lf: bool;
+            Err(e) => {
-
-    // reading from a TTY seems to raise a condition on, rather than return Some(0) like a file.
-    // hence the option wrapped in a result here
-    let mut buffered_reader = reader.get_buffered();
-    loop {
-        match buffered_reader.read_until(LF, &mut raw_line) {
-            Ok(n) => {
-                if n == 0 {
-                    break;
-                }
-            }
-            Err(ref e) => {
-                if !raw_line.is_empty() {
                 show_warning!("Error while reading {}: {}", path, e);
-                } else {
+                None
-                    break;
             }
-            }
-        };
-
-        // GNU 'wc' only counts lines that end in LF as lines
-        ends_lf = *raw_line.last().unwrap() == LF;
-        line_count += ends_lf as usize;
-
-        byte_count += raw_line.len();
-
-        if decode_chars {
-            // try and convert the bytes to UTF-8 first
-            let current_char_count;
-            match from_utf8(&raw_line[..]) {
-                Ok(line) => {
-                    word_count += line.split_whitespace().count();
-                    current_char_count = line.chars().count();
-                }
-                Err(..) => {
-                    word_count += raw_line.split(|&x| is_word_separator(x)).count();
-                    current_char_count = raw_line.iter().filter(|c| c.is_ascii()).count()
-                }
-            }
-            char_count += current_char_count;
-            if current_char_count > longest_line_length {
-                // -L is a GNU 'wc' extension so same behavior on LF
-                longest_line_length = current_char_count - (ends_lf as usize);
-            }
-        }
-
-        raw_line.truncate(0);
-    }
-
-    Ok(WordCount {
-        bytes: byte_count,
-        chars: char_count,
-        lines: line_count,
-        words: word_count,
-        max_line_length: longest_line_length,
         })
+        .map(|line| WordCount::from_line(&line, decode_chars))
+        .sum();
+    Ok(total)
 }
 
 fn word_count_from_path(path: &str, settings: &Settings) -> WcResult<WordCount> {
@@ -360,7 +228,12 @@ fn wc(files: Vec<String>, settings: &Settings) -> Result<(), u32> {
             error_count += 1;
             WordCount::default()
         });
-        max_width = max(max_width, word_count.bytes.to_string().len() + 1);
+        // Compute the number of digits needed to display the number
+        // of bytes in the file. Even if the settings indicate that we
+        // won't *display* the number of bytes, we still use the
+        // number of digits in the byte count as the width when
+        // formatting each count as a string for output.
+        max_width = max(max_width, word_count.bytes.to_string().len());
         total_word_count += word_count;
         results.push(word_count.with_title(path));
     }
@@ -401,19 +274,40 @@ fn print_stats(
         min_width = 0;
     }
 
+    let mut is_first: bool = true;
+
     if settings.show_lines {
+        if !is_first {
+            write!(stdout_lock, " ")?;
+        }
         write!(stdout_lock, "{:1$}", result.count.lines, min_width)?;
+        is_first = false;
     }
     if settings.show_words {
+        if !is_first {
+            write!(stdout_lock, " ")?;
+        }
         write!(stdout_lock, "{:1$}", result.count.words, min_width)?;
+        is_first = false;
     }
     if settings.show_bytes {
+        if !is_first {
+            write!(stdout_lock, " ")?;
+        }
         write!(stdout_lock, "{:1$}", result.count.bytes, min_width)?;
+        is_first = false;
     }
     if settings.show_chars {
+        if !is_first {
+            write!(stdout_lock, " ")?;
+        }
         write!(stdout_lock, "{:1$}", result.count.chars, min_width)?;
+        is_first = false;
     }
     if settings.show_max_line_length {
+        if !is_first {
+            write!(stdout_lock, " ")?;
+        }
         write!(
             stdout_lock,
             "{:1$}",

src/uu/wc/src/wordcount.rs

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

src/uu/who/Cargo.toml

@@ -17,6 +17,7 @@ path = "src/who.rs"
 [dependencies]
 uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["utmpx"] }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
+clap = "2.33.3"
 
 [[bin]]
 name = "who"

src/uu/who/src/who.rs

@@ -12,79 +12,169 @@ extern crate uucore;
 use uucore::libc::{ttyname, STDIN_FILENO, S_IWGRP};
 use uucore::utmpx::{self, time, Utmpx};
 
+use clap::{App, Arg};
 use std::borrow::Cow;
 use std::ffi::CStr;
 use std::os::unix::fs::MetadataExt;
 use std::path::PathBuf;
 use uucore::InvalidEncodingHandling;
 
-static SYNTAX: &str = "[OPTION]... [ FILE | ARG1 ARG2 ]";
+mod options {
-static SUMMARY: &str = "Print information about users who are currently logged in.";
+    pub const ALL: &str = "all";
-static LONG_HELP: &str = "
+    pub const BOOT: &str = "boot";
-  -a, --all         same as -b -d --login -p -r -t -T -u
+    pub const DEAD: &str = "dead";
-  -b, --boot        time of last system boot
+    pub const HEADING: &str = "heading";
-  -d, --dead        print dead processes
+    pub const LOGIN: &str = "login";
-  -H, --heading     print line of column headings
+    pub const LOOKUP: &str = "lookup";
-  -l, --login       print system login processes
+    pub const ONLY_HOSTNAME_USER: &str = "only_hostname_user";
-      --lookup      attempt to canonicalize hostnames via DNS
+    pub const PROCESS: &str = "process";
-  -m                only hostname and user associated with stdin
+    pub const COUNT: &str = "count";
-  -p, --process     print active processes spawned by init
+    #[cfg(any(target_vendor = "apple", target_os = "linux", target_os = "android"))]
-  -q, --count       all login names and number of users logged on
+    pub const RUNLEVEL: &str = "runlevel";
-  -r, --runlevel    print current runlevel (not available on BSDs)
+    pub const SHORT: &str = "short";
-  -s, --short       print only name, line, and time (default)
+    pub const TIME: &str = "time";
-  -t, --time        print last system clock change
+    pub const USERS: &str = "users";
-  -T, -w, --mesg    add user's message status as +, - or ?
+    pub const MESG: &str = "mesg"; // aliases: --message, --writable
-  -u, --users       list users logged in
+    pub const FILE: &str = "FILE"; // if length=1: FILE, if length=2: ARG1 ARG2
-      --message     same as -T
+}
-      --writable    same as -T
-      --help     display this help and exit
-      --version  output version information and exit
 
-If FILE is not specified, use /var/run/utmp.  /var/log/wtmp as FILE is common.
+static VERSION: &str = env!("CARGO_PKG_VERSION");
-If ARG1 ARG2 given, -m presumed: 'am i' or 'mom likes' are usual.
+static ABOUT: &str = "Print information about users who are currently logged in.";
-";
+
+fn get_usage() -> String {
+    format!("{0} [OPTION]... [ FILE | ARG1 ARG2 ]", executable!())
+}
+
+fn get_long_usage() -> String {
+    String::from(
+        "If FILE is not specified, use /var/run/utmp.  /var/log/wtmp as FILE is common.\n\
+If ARG1 ARG2 given, -m presumed: 'am i' or 'mom likes' are usual.",
+    )
+}
 
 pub fn uumain(args: impl uucore::Args) -> i32 {
     let args = args
         .collect_str(InvalidEncodingHandling::Ignore)
         .accept_any();
 
-    let mut opts = app!(SYNTAX, SUMMARY, LONG_HELP);
+    let usage = get_usage();
-    opts.optflag("a", "all", "same as -b -d --login -p -r -t -T -u");
+    let after_help = get_long_usage();
-    opts.optflag("b", "boot", "time of last system boot");
+
-    opts.optflag("d", "dead", "print dead processes");
+    let matches = App::new(executable!())
-    opts.optflag("H", "heading", "print line of column headings");
+        .version(VERSION)
-    opts.optflag("l", "login", "print system login processes");
+        .about(ABOUT)
-    opts.optflag("", "lookup", "attempt to canonicalize hostnames via DNS");
+        .usage(&usage[..])
-    opts.optflag("m", "", "only hostname and user associated with stdin");
+        .after_help(&after_help[..])
-    opts.optflag("p", "process", "print active processes spawned by init");
+        .arg(
-    opts.optflag(
+            Arg::with_name(options::ALL)
-        "q",
+                .long(options::ALL)
-        "count",
+                .short("a")
-        "all login names and number of users logged on",
+                .help("same as -b -d --login -p -r -t -T -u"),
-    );
+        )
+        .arg(
+            Arg::with_name(options::BOOT)
+                .long(options::BOOT)
+                .short("b")
+                .help("time of last system boot"),
+        )
+        .arg(
+            Arg::with_name(options::DEAD)
+                .long(options::DEAD)
+                .short("d")
+                .help("print dead processes"),
+        )
+        .arg(
+            Arg::with_name(options::HEADING)
+                .long(options::HEADING)
+                .short("H")
+                .help("print line of column headings"),
+        )
+        .arg(
+            Arg::with_name(options::LOGIN)
+                .long(options::LOGIN)
+                .short("l")
+                .help("print system login processes"),
+        )
+        .arg(
+            Arg::with_name(options::LOOKUP)
+                .long(options::LOOKUP)
+                .help("attempt to canonicalize hostnames via DNS"),
+        )
+        .arg(
+            Arg::with_name(options::ONLY_HOSTNAME_USER)
+                .short("m")
+                .help("only hostname and user associated with stdin"),
+        )
+        .arg(
+            Arg::with_name(options::PROCESS)
+                .long(options::PROCESS)
+                .short("p")
+                .help("print active processes spawned by init"),
+        )
+        .arg(
+            Arg::with_name(options::COUNT)
+                .long(options::COUNT)
+                .short("q")
+                .help("all login names and number of users logged on"),
+        )
+        .arg(
             #[cfg(any(target_vendor = "apple", target_os = "linux", target_os = "android"))]
-    opts.optflag("r", "runlevel", "print current runlevel");
+            Arg::with_name(options::RUNLEVEL)
-    opts.optflag("s", "short", "print only name, line, and time (default)");
+                .long(options::RUNLEVEL)
-    opts.optflag("t", "time", "print last system clock change");
+                .short("r")
-    opts.optflag("u", "users", "list users logged in");
+                .help("print current runlevel"),
-    opts.optflag("w", "mesg", "add user's message status as +, - or ?");
+        )
-    // --message, --writable are the same as --mesg
+        .arg(
-    opts.optflag("T", "message", "");
+            Arg::with_name(options::SHORT)
-    opts.optflag("T", "writable", "");
+                .long(options::SHORT)
+                .short("s")
+                .help("print only name, line, and time (default)"),
+        )
+        .arg(
+            Arg::with_name(options::TIME)
+                .long(options::TIME)
+                .short("t")
+                .help("print last system clock change"),
+        )
+        .arg(
+            Arg::with_name(options::USERS)
+                .long(options::USERS)
+                .short("u")
+                .help("list users logged in"),
+        )
+        .arg(
+            Arg::with_name(options::MESG)
+                .long(options::MESG)
+                .short("T")
+                // .visible_short_alias('w')  // TODO: requires clap "3.0.0-beta.2"
+                .visible_aliases(&["message", "writable"])
+                .help("add user's message status as +, - or ?"),
+        )
+        .arg(
+            Arg::with_name("w") // work around for `Arg::visible_short_alias`
+                .short("w")
+                .help("same as -T"),
+        )
+        .arg(
+            Arg::with_name(options::FILE)
+                .takes_value(true)
+                .min_values(1)
+                .max_values(2),
+        )
+        .get_matches_from(args);
 
-    opts.optflag("", "help", "display this help and exit");
+    let files: Vec<String> = matches
-    opts.optflag("", "version", "output version information and exit");
+        .values_of(options::FILE)
-
+        .map(|v| v.map(ToString::to_string).collect())
-    let matches = opts.parse(args);
+        .unwrap_or_default();
 
     // If true, attempt to canonicalize hostnames via a DNS lookup.
-    let do_lookup = matches.opt_present("lookup");
+    let do_lookup = matches.is_present(options::LOOKUP);
 
     // If true, display only a list of usernames and count of
     // the users logged on.
     // Ignored for 'who am i'.
-    let short_list = matches.opt_present("q");
+    let short_list = matches.is_present(options::COUNT);
 
     // If true, display only name, line, and time fields.
     let mut short_output = false;
@@ -95,12 +185,13 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
     let mut include_idle = false;
 
     // If true, display a line at the top describing each field.
-    let include_heading = matches.opt_present("H");
+    let include_heading = matches.is_present(options::HEADING);
 
     // If true, display a '+' for each user if mesg y, a '-' if mesg n,
     // or a '?' if their tty cannot be statted.
-    let include_mesg =
+    let include_mesg = matches.is_present(options::ALL)
-        matches.opt_present("a") || matches.opt_present("T") || matches.opt_present("w");
+        || matches.is_present(options::MESG)
+        || matches.is_present("w");
 
     // If true, display process termination & exit status.
     let mut include_exit = false;
@@ -133,7 +224,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
 
     #[allow(clippy::useless_let_if_seq)]
     {
-        if matches.opt_present("a") {
+        if matches.is_present(options::ALL) {
             need_boottime = true;
             need_deadprocs = true;
             need_login = true;
@@ -146,49 +237,49 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
             assumptions = false;
         }
 
-        if matches.opt_present("b") {
+        if matches.is_present(options::BOOT) {
             need_boottime = true;
             assumptions = false;
         }
 
-        if matches.opt_present("d") {
+        if matches.is_present(options::DEAD) {
             need_deadprocs = true;
             include_idle = true;
             include_exit = true;
             assumptions = false;
         }
 
-        if matches.opt_present("l") {
+        if matches.is_present(options::LOGIN) {
             need_login = true;
             include_idle = true;
             assumptions = false;
         }
 
-        if matches.opt_present("m") || matches.free.len() == 2 {
+        if matches.is_present(options::ONLY_HOSTNAME_USER) || files.len() == 2 {
             my_line_only = true;
         }
 
-        if matches.opt_present("p") {
+        if matches.is_present(options::PROCESS) {
             need_initspawn = true;
             assumptions = false;
         }
 
-        if matches.opt_present("r") {
+        if matches.is_present(options::RUNLEVEL) {
             need_runlevel = true;
             include_idle = true;
             assumptions = false;
         }
 
-        if matches.opt_present("s") {
+        if matches.is_present(options::SHORT) {
             short_output = true;
         }
 
-        if matches.opt_present("t") {
+        if matches.is_present(options::TIME) {
             need_clockchange = true;
             assumptions = false;
         }
 
-        if matches.opt_present("u") {
+        if matches.is_present(options::USERS) {
             need_users = true;
             include_idle = true;
             assumptions = false;
@@ -202,11 +293,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         if include_exit {
             short_output = false;
         }
-
-        if matches.free.len() > 2 {
-            show_usage_error!("{}", msg_wrong_number_of_arguments!());
-            exit!(1);
-        }
     }
 
     let mut who = Who {
@@ -225,7 +311,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
         need_runlevel,
         need_users,
         my_line_only,
-        args: matches.free,
+        args: files,
     };
 
     who.exec();

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

@@ -8,8 +8,9 @@
 
 #[cfg(unix)]
 use libc::{
-    mode_t, S_IRGRP, S_IROTH, S_IRUSR, S_ISGID, S_ISUID, S_ISVTX, S_IWGRP, S_IWOTH, S_IWUSR,
+    mode_t, S_IFBLK, S_IFCHR, S_IFDIR, S_IFIFO, S_IFLNK, S_IFMT, S_IFREG, S_IFSOCK, S_IRGRP,
-    S_IXGRP, S_IXOTH, S_IXUSR,
+    S_IROTH, S_IRUSR, S_ISGID, S_ISUID, S_ISVTX, S_IWGRP, S_IWOTH, S_IWUSR, S_IXGRP, S_IXOTH,
+    S_IXUSR,
 };
 use std::borrow::Cow;
 use std::env;
@@ -23,9 +24,10 @@ use std::os::unix::fs::MetadataExt;
 use std::path::{Component, Path, PathBuf};
 
 #[cfg(unix)]
+#[macro_export]
 macro_rules! has {
     ($mode:expr, $perm:expr) => {
-        $mode & ($perm as u32) != 0
+        $mode & $perm != 0
     };
 }
 
@@ -240,22 +242,42 @@ pub fn is_stderr_interactive() -> bool {
 
 #[cfg(not(unix))]
 #[allow(unused_variables)]
-pub fn display_permissions(metadata: &fs::Metadata) -> String {
+pub fn display_permissions(metadata: &fs::Metadata, display_file_type: bool) -> String {
+    if display_file_type {
+        return String::from("----------");
+    }
     String::from("---------")
 }
 
 #[cfg(unix)]
-pub fn display_permissions(metadata: &fs::Metadata) -> String {
+pub fn display_permissions(metadata: &fs::Metadata, display_file_type: bool) -> String {
     let mode: mode_t = metadata.mode() as mode_t;
-    display_permissions_unix(mode as u32)
+    display_permissions_unix(mode, display_file_type)
 }
 
 #[cfg(unix)]
-pub fn display_permissions_unix(mode: u32) -> String {
+pub fn display_permissions_unix(mode: mode_t, display_file_type: bool) -> String {
-    let mut result = String::with_capacity(9);
+    let mut result;
+    if display_file_type {
+        result = String::with_capacity(10);
+        result.push(match mode & S_IFMT {
+            S_IFDIR => 'd',
+            S_IFCHR => 'c',
+            S_IFBLK => 'b',
+            S_IFREG => '-',
+            S_IFIFO => 'p',
+            S_IFLNK => 'l',
+            S_IFSOCK => 's',
+            // TODO: Other file types
+            _ => '?',
+        });
+    } else {
+        result = String::with_capacity(9);
+    }
+
     result.push(if has!(mode, S_IRUSR) { 'r' } else { '-' });
     result.push(if has!(mode, S_IWUSR) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISUID) {
+    result.push(if has!(mode, S_ISUID as mode_t) {
         if has!(mode, S_IXUSR) {
             's'
         } else {
@@ -269,7 +291,7 @@ pub fn display_permissions_unix(mode: u32) -> String {
 
     result.push(if has!(mode, S_IRGRP) { 'r' } else { '-' });
     result.push(if has!(mode, S_IWGRP) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISGID) {
+    result.push(if has!(mode, S_ISGID as mode_t) {
         if has!(mode, S_IXGRP) {
             's'
         } else {
@@ -283,7 +305,7 @@ pub fn display_permissions_unix(mode: u32) -> String {
 
     result.push(if has!(mode, S_IROTH) { 'r' } else { '-' });
     result.push(if has!(mode, S_IWOTH) { 'w' } else { '-' });
-    result.push(if has!(mode, S_ISVTX) {
+    result.push(if has!(mode, S_ISVTX as mode_t) {
         if has!(mode, S_IXOTH) {
             't'
         } else {
@@ -355,4 +377,57 @@ mod tests {
             );
         }
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn test_display_permissions() {
+        assert_eq!(
+            "drwxr-xr-x",
+            display_permissions_unix(S_IFDIR | 0o755, true)
+        );
+        assert_eq!(
+            "rwxr-xr-x",
+            display_permissions_unix(S_IFDIR | 0o755, false)
+        );
+        assert_eq!(
+            "-rw-r--r--",
+            display_permissions_unix(S_IFREG | 0o644, true)
+        );
+        assert_eq!(
+            "srw-r-----",
+            display_permissions_unix(S_IFSOCK | 0o640, true)
+        );
+        assert_eq!(
+            "lrw-r-xr-x",
+            display_permissions_unix(S_IFLNK | 0o655, true)
+        );
+        assert_eq!("?rw-r-xr-x", display_permissions_unix(0o655, true));
+
+        assert_eq!(
+            "brwSr-xr-x",
+            display_permissions_unix(S_IFBLK | S_ISUID as mode_t | 0o655, true)
+        );
+        assert_eq!(
+            "brwsr-xr-x",
+            display_permissions_unix(S_IFBLK | S_ISUID as mode_t | 0o755, true)
+        );
+
+        assert_eq!(
+            "prw---sr--",
+            display_permissions_unix(S_IFIFO | S_ISGID as mode_t | 0o614, true)
+        );
+        assert_eq!(
+            "prw---Sr--",
+            display_permissions_unix(S_IFIFO | S_ISGID as mode_t | 0o604, true)
+        );
+
+        assert_eq!(
+            "c---r-xr-t",
+            display_permissions_unix(S_IFCHR | S_ISVTX as mode_t | 0o055, true)
+        );
+        assert_eq!(
+            "c---r-xr-T",
+            display_permissions_unix(S_IFCHR | S_ISVTX as mode_t | 0o054, true)
+        );
+    }
 }

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

@@ -132,19 +132,15 @@ fn parse_change(mode: &str, fperm: u32, considering_dir: bool) -> (u32, usize) {
     (srwx, pos)
 }
 
-pub fn parse_mode(mode: Option<String>) -> Result<mode_t, String> {
+pub fn parse_mode(mode: &str) -> Result<mode_t, String> {
     let fperm = S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH;
-    if let Some(mode) = mode {
     let arr: &[char] = &['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'];
     let result = if mode.contains(arr) {
-            parse_numeric(fperm as u32, mode.as_str())
+        parse_numeric(fperm as u32, mode)
     } else {
-            parse_symbolic(fperm as u32, mode.as_str(), true)
+        parse_symbolic(fperm as u32, mode, true)
     };
     result.map(|mode| mode as mode_t)
-    } else {
-        Ok(fperm)
-    }
 }
 
 #[cfg(test)]
@@ -152,20 +148,19 @@ mod test {
 
     #[test]
     fn symbolic_modes() {
-        assert_eq!(super::parse_mode(Some("u+x".to_owned())).unwrap(), 0o766);
+        assert_eq!(super::parse_mode("u+x").unwrap(), 0o766);
         assert_eq!(
-            super::parse_mode(Some("+x".to_owned())).unwrap(),
+            super::parse_mode("+x").unwrap(),
             if !crate::os::is_wsl_1() { 0o777 } else { 0o776 }
         );
-        assert_eq!(super::parse_mode(Some("a-w".to_owned())).unwrap(), 0o444);
+        assert_eq!(super::parse_mode("a-w").unwrap(), 0o444);
-        assert_eq!(super::parse_mode(Some("g-r".to_owned())).unwrap(), 0o626);
+        assert_eq!(super::parse_mode("g-r").unwrap(), 0o626);
     }
 
     #[test]
     fn numeric_modes() {
-        assert_eq!(super::parse_mode(Some("644".to_owned())).unwrap(), 0o644);
+        assert_eq!(super::parse_mode("644").unwrap(), 0o644);
-        assert_eq!(super::parse_mode(Some("+100".to_owned())).unwrap(), 0o766);
+        assert_eq!(super::parse_mode("+100").unwrap(), 0o766);
-        assert_eq!(super::parse_mode(Some("-4".to_owned())).unwrap(), 0o662);
+        assert_eq!(super::parse_mode("-4").unwrap(), 0o662);
-        assert_eq!(super::parse_mode(None).unwrap(), 0o666);
     }
 }

tests/by-util/test_basename.rs

@@ -1,6 +1,29 @@
 use crate::common::util::*;
 use std::ffi::OsStr;
 
+#[test]
+fn test_help() {
+    for help_flg in vec!["-h", "--help"] {
+        new_ucmd!()
+            .arg(&help_flg)
+            .succeeds()
+            .no_stderr()
+            .stdout_contains("USAGE:");
+    }
+}
+
+#[test]
+fn test_version() {
+    for version_flg in vec!["-V", "--version"] {
+        assert!(new_ucmd!()
+            .arg(&version_flg)
+            .succeeds()
+            .no_stderr()
+            .stdout_str()
+            .starts_with("basename"));
+    }
+}
+
 #[test]
 fn test_directory() {
     new_ucmd!()
@@ -81,11 +104,25 @@ fn test_no_args() {
     expect_error(vec![]);
 }
 
+#[test]
+fn test_no_args_output() {
+    new_ucmd!()
+        .fails()
+        .stderr_is("basename: error: missing operand\nTry 'basename --help' for more information.");
+}
+
 #[test]
 fn test_too_many_args() {
     expect_error(vec!["a", "b", "c"]);
 }
 
+#[test]
+fn test_too_many_args_output() {
+    new_ucmd!().args(&["a", "b", "c"]).fails().stderr_is(
+        "basename: error: extra operand 'c'\nTry 'basename --help' for more information.",
+    );
+}
+
 fn test_invalid_utf8_args(os_str: &OsStr) {
     let test_vec = vec![os_str.to_os_string()];
     new_ucmd!().args(&test_vec).succeeds().stdout_is("fo<EFBFBD>o\n");

tests/by-util/test_df.rs

@@ -20,4 +20,16 @@ fn test_df_compatible_si() {
     new_ucmd!().arg("-aH").succeeds();
 }
 
+#[test]
+fn test_df_output() {
+    if cfg!(target_os = "macos") {
+        new_ucmd!().arg("-H").arg("-total").succeeds().
+        stdout_only("Filesystem               Size         Used    Available     Capacity  Use% Mounted on       \n");
+    } else {
+        new_ucmd!().arg("-H").arg("-total").succeeds().stdout_only(
+            "Filesystem               Size         Used    Available  Use% Mounted on       \n",
+        );
+    }
+}
+
 // ToDO: more tests...

tests/by-util/test_du.rs

@@ -53,7 +53,15 @@ fn _du_basics_subdir(s: &str) {
 fn _du_basics_subdir(s: &str) {
     assert_eq!(s, "0\tsubdir/deeper\n");
 }
-#[cfg(all(not(target_vendor = "apple"), not(target_os = "windows")))]
+#[cfg(target_os = "freebsd")]
+fn _du_basics_subdir(s: &str) {
+    assert_eq!(s, "8\tsubdir/deeper\n");
+}
+#[cfg(all(
+    not(target_vendor = "apple"),
+    not(target_os = "windows"),
+    not(target_os = "freebsd")
+))]
 fn _du_basics_subdir(s: &str) {
     // MS-WSL linux has altered expected output
     if !uucore::os::is_wsl_1() {
@@ -100,7 +108,15 @@ fn _du_soft_link(s: &str) {
 fn _du_soft_link(s: &str) {
     assert_eq!(s, "8\tsubdir/links\n");
 }
-#[cfg(all(not(target_vendor = "apple"), not(target_os = "windows")))]
+#[cfg(target_os = "freebsd")]
+fn _du_soft_link(s: &str) {
+    assert_eq!(s, "16\tsubdir/links\n");
+}
+#[cfg(all(
+    not(target_vendor = "apple"),
+    not(target_os = "windows"),
+    not(target_os = "freebsd")
+))]
 fn _du_soft_link(s: &str) {
     // MS-WSL linux has altered expected output
     if !uucore::os::is_wsl_1() {
@@ -141,7 +157,15 @@ fn _du_hard_link(s: &str) {
 fn _du_hard_link(s: &str) {
     assert_eq!(s, "8\tsubdir/links\n")
 }
-#[cfg(all(not(target_vendor = "apple"), not(target_os = "windows")))]
+#[cfg(target_os = "freebsd")]
+fn _du_hard_link(s: &str) {
+    assert_eq!(s, "16\tsubdir/links\n")
+}
+#[cfg(all(
+    not(target_vendor = "apple"),
+    not(target_os = "windows"),
+    not(target_os = "freebsd")
+))]
 fn _du_hard_link(s: &str) {
     // MS-WSL linux has altered expected output
     if !uucore::os::is_wsl_1() {
@@ -181,7 +205,15 @@ fn _du_d_flag(s: &str) {
 fn _du_d_flag(s: &str) {
     assert_eq!(s, "8\t./subdir\n8\t./\n");
 }
-#[cfg(all(not(target_vendor = "apple"), not(target_os = "windows")))]
+#[cfg(target_os = "freebsd")]
+fn _du_d_flag(s: &str) {
+    assert_eq!(s, "28\t./subdir\n36\t./\n");
+}
+#[cfg(all(
+    not(target_vendor = "apple"),
+    not(target_os = "windows"),
+    not(target_os = "freebsd")
+))]
 fn _du_d_flag(s: &str) {
     // MS-WSL linux has altered expected output
     if !uucore::os::is_wsl_1() {

tests/by-util/test_fmt.rs

@@ -33,8 +33,7 @@ fn test_fmt_w_too_big() {
         "fmt: error: invalid width: '2501': Numerical result out of range"
     );
 }
-/* #[test]
+#[test]
- Fails for now, see https://github.com/uutils/coreutils/issues/1501
 fn test_fmt_w() {
     let result = new_ucmd!()
         .arg("-w")
@@ -42,9 +41,8 @@ fn test_fmt_w() {
         .arg("one-word-per-line.txt")
         .run();
     //.stdout_is_fixture("call_graph.expected");
-    assert_eq!(result.stdout_str().trim(), "this is a file with one word per line");
+    assert_eq!(
+        result.stdout_str().trim(),
+        "this is\na file\nwith one\nword per\nline"
+    );
 }
-
-
-fmt is pretty broken in general, needs more works to have more tests
- */

tests/by-util/test_ls.rs

@@ -5,6 +5,7 @@ use crate::common::util::*;
 extern crate regex;
 use self::regex::Regex;
 
+use std::collections::HashMap;
 use std::path::Path;
 use std::thread::sleep;
 use std::time::Duration;
@@ -308,6 +309,50 @@ fn test_ls_long() {
     }
 }
 
+#[test]
+fn test_ls_long_total_size() {
+    let scene = TestScenario::new(util_name!());
+    let at = &scene.fixtures;
+    at.touch(&at.plus_as_string("test-long"));
+    at.append("test-long", "1");
+    at.touch(&at.plus_as_string("test-long2"));
+    at.append("test-long2", "2");
+
+    let expected_prints: HashMap<_, _> = if cfg!(unix) {
+        [
+            ("long_vanilla", "total 8"),
+            ("long_human_readable", "total 8.0K"),
+            ("long_si", "total 8.2k"),
+        ]
+        .iter()
+        .cloned()
+        .collect()
+    } else {
+        [
+            ("long_vanilla", "total 2"),
+            ("long_human_readable", "total 2"),
+            ("long_si", "total 2"),
+        ]
+        .iter()
+        .cloned()
+        .collect()
+    };
+
+    for arg in &["-l", "--long", "--format=long", "--format=verbose"] {
+        let result = scene.ucmd().arg(arg).succeeds();
+        result.stdout_contains(expected_prints["long_vanilla"]);
+
+        for arg2 in &["-h", "--human-readable", "--si"] {
+            let result = scene.ucmd().arg(arg).arg(arg2).succeeds();
+            result.stdout_contains(if *arg2 == "--si" {
+                expected_prints["long_si"]
+            } else {
+                expected_prints["long_human_readable"]
+            });
+        }
+    }
+}
+
 #[test]
 fn test_ls_long_formats() {
     let scene = TestScenario::new(util_name!());

tests/by-util/test_mknod.rs

@@ -1 +1,124 @@
-// ToDO: add tests
+use crate::common::util::*;
+
+#[cfg(not(windows))]
+#[test]
+fn test_mknod_help() {
+    new_ucmd!()
+        .arg("--help")
+        .succeeds()
+        .no_stderr()
+        .stdout_contains("USAGE:");
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_mknod_version() {
+    assert!(new_ucmd!()
+        .arg("--version")
+        .succeeds()
+        .no_stderr()
+        .stdout_str()
+        .starts_with("mknod"));
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_mknod_fifo_default_writable() {
+    let ts = TestScenario::new(util_name!());
+    ts.ucmd().arg("test_file").arg("p").succeeds();
+    assert!(ts.fixtures.is_fifo("test_file"));
+    assert!(!ts.fixtures.metadata("test_file").permissions().readonly());
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_mknod_fifo_mnemonic_usage() {
+    let ts = TestScenario::new(util_name!());
+    ts.ucmd().arg("test_file").arg("pipe").succeeds();
+    assert!(ts.fixtures.is_fifo("test_file"));
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_mknod_fifo_read_only() {
+    let ts = TestScenario::new(util_name!());
+    ts.ucmd()
+        .arg("-m")
+        .arg("a=r")
+        .arg("test_file")
+        .arg("p")
+        .succeeds();
+    assert!(ts.fixtures.is_fifo("test_file"));
+    assert!(ts.fixtures.metadata("test_file").permissions().readonly());
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_mknod_fifo_invalid_extra_operand() {
+    new_ucmd!()
+        .arg("test_file")
+        .arg("p")
+        .arg("1")
+        .arg("2")
+        .fails()
+        .stderr_contains(&"Fifos do not have major and minor device numbers");
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_mknod_character_device_requires_major_and_minor() {
+    new_ucmd!()
+        .arg("test_file")
+        .arg("c")
+        .fails()
+        .status_code(1)
+        .stderr_contains(&"Special files require major and minor device numbers.");
+    new_ucmd!()
+        .arg("test_file")
+        .arg("c")
+        .arg("1")
+        .fails()
+        .status_code(1)
+        .stderr_contains(&"Special files require major and minor device numbers.");
+    new_ucmd!()
+        .arg("test_file")
+        .arg("c")
+        .arg("1")
+        .arg("c")
+        .fails()
+        .status_code(1)
+        .stderr_contains(&"Invalid value for '<MINOR>'");
+    new_ucmd!()
+        .arg("test_file")
+        .arg("c")
+        .arg("c")
+        .arg("1")
+        .fails()
+        .status_code(1)
+        .stderr_contains(&"Invalid value for '<MAJOR>'");
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_mknod_invalid_arg() {
+    new_ucmd!()
+        .arg("--foo")
+        .fails()
+        .status_code(1)
+        .no_stdout()
+        .stderr_contains(&"Found argument '--foo' which wasn't expected");
+}
+
+#[test]
+#[cfg(not(windows))]
+fn test_mknod_invalid_mode() {
+    new_ucmd!()
+        .arg("--mode")
+        .arg("rw")
+        .arg("test_file")
+        .arg("p")
+        .fails()
+        .no_stdout()
+        .status_code(1)
+        .stderr_contains(&"invalid mode");
+}

tests/by-util/test_sort.rs

@@ -37,7 +37,29 @@ fn test_larger_than_specified_segment() {
         .arg("50K")
         .arg("ext_sort.txt")
         .succeeds()
-        .stdout_is_fixture(format!("{}", "ext_sort.expected"));
+        .stdout_is_fixture("ext_sort.expected");
+}
+
+#[test]
+fn test_smaller_than_specified_segment() {
+    new_ucmd!()
+        .arg("-n")
+        .arg("-S")
+        .arg("100M")
+        .arg("ext_sort.txt")
+        .succeeds()
+        .stdout_is_fixture("ext_sort.expected");
+}
+
+#[test]
+fn test_extsort_zero_terminated() {
+    new_ucmd!()
+        .arg("-z")
+        .arg("-S")
+        .arg("10K")
+        .arg("zero-terminated.txt")
+        .succeeds()
+        .stdout_is_fixture("zero-terminated.expected");
 }
 
 #[test]

tests/by-util/test_split.rs

@@ -4,11 +4,15 @@ extern crate regex;
 use self::rand::{thread_rng, Rng};
 use self::regex::Regex;
 use crate::common::util::*;
+use rand::SeedableRng;
 #[cfg(not(windows))]
 use std::env;
-use std::fs::{read_dir, File};
 use std::io::Write;
 use std::path::Path;
+use std::{
+    fs::{read_dir, File},
+    io::BufWriter,
+};
 
 fn random_chars(n: usize) -> String {
     thread_rng()
@@ -58,7 +62,7 @@ impl Glob {
         files.sort();
         let mut data: Vec<u8> = vec![];
         for name in &files {
-            data.extend(self.directory.read(name).into_bytes());
+            data.extend(self.directory.read_bytes(name));
         }
         data
     }
@@ -81,20 +85,30 @@ impl RandomFile {
     }
 
     fn add_bytes(&mut self, bytes: usize) {
-        let chunk_size: usize = if bytes >= 1024 { 1024 } else { bytes };
+        // Note that just writing random characters isn't enough to cover all
-        let mut n = bytes;
+        // cases. We need truly random bytes.
-        while n > chunk_size {
+        let mut writer = BufWriter::new(&self.inner);
-            let _ = write!(self.inner, "{}", random_chars(chunk_size));
+
-            n -= chunk_size;
+        // Seed the rng so as to avoid spurious test failures.
+        let mut rng = rand::rngs::StdRng::seed_from_u64(123);
+        let mut buffer = [0; 1024];
+        let mut remaining_size = bytes;
+
+        while remaining_size > 0 {
+            let to_write = std::cmp::min(remaining_size, buffer.len());
+            let buf = &mut buffer[..to_write];
+            rng.fill(buf);
+            writer.write(buf).unwrap();
+
+            remaining_size -= to_write;
         }
-        let _ = write!(self.inner, "{}", random_chars(n));
     }
 
     /// Add n lines each of size `RandomFile::LINESIZE`
     fn add_lines(&mut self, lines: usize) {
         let mut n = lines;
         while n > 0 {
-            let _ = writeln!(self.inner, "{}", random_chars(RandomFile::LINESIZE));
+            writeln!(self.inner, "{}", random_chars(RandomFile::LINESIZE)).unwrap();
             n -= 1;
         }
     }
@@ -104,18 +118,18 @@ impl RandomFile {
 fn test_split_default() {
     let (at, mut ucmd) = at_and_ucmd!();
     let name = "split_default";
-    let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$");
     RandomFile::new(&at, name).add_lines(2000);
     ucmd.args(&[name]).succeeds();
+
+    let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$");
     assert_eq!(glob.count(), 2);
-    assert_eq!(glob.collate(), at.read(name).into_bytes());
+    assert_eq!(glob.collate(), at.read_bytes(name));
 }
 
 #[test]
 fn test_split_numeric_prefixed_chunks_by_bytes() {
     let (at, mut ucmd) = at_and_ucmd!();
     let name = "split_num_prefixed_chunks_by_bytes";
-    let glob = Glob::new(&at, ".", r"a\d\d$");
     RandomFile::new(&at, name).add_bytes(10000);
     ucmd.args(&[
         "-d", // --numeric-suffixes
@@ -123,52 +137,89 @@ fn test_split_numeric_prefixed_chunks_by_bytes() {
         "1000", name, "a",
     ])
     .succeeds();
+
+    let glob = Glob::new(&at, ".", r"a\d\d$");
     assert_eq!(glob.count(), 10);
-    assert_eq!(glob.collate(), at.read(name).into_bytes());
+    for filename in glob.collect() {
+        assert_eq!(glob.directory.metadata(&filename).len(), 1000);
+    }
+    assert_eq!(glob.collate(), at.read_bytes(name));
 }
 
 #[test]
 fn test_split_str_prefixed_chunks_by_bytes() {
     let (at, mut ucmd) = at_and_ucmd!();
     let name = "split_str_prefixed_chunks_by_bytes";
-    let glob = Glob::new(&at, ".", r"b[[:alpha:]][[:alpha:]]$");
     RandomFile::new(&at, name).add_bytes(10000);
+    // Important that this is less than 1024 since that's our internal buffer
+    // size. Good to test that we don't overshoot.
     ucmd.args(&["-b", "1000", name, "b"]).succeeds();
+
+    let glob = Glob::new(&at, ".", r"b[[:alpha:]][[:alpha:]]$");
     assert_eq!(glob.count(), 10);
-    assert_eq!(glob.collate(), at.read(name).into_bytes());
+    for filename in glob.collect() {
+        assert_eq!(glob.directory.metadata(&filename).len(), 1000);
+    }
+    assert_eq!(glob.collate(), at.read_bytes(name));
+}
+
+// This is designed to test what happens when the desired part size is not a
+// multiple of the buffer size and we hopefully don't overshoot the desired part
+// size.
+#[test]
+fn test_split_bytes_prime_part_size() {
+    let (at, mut ucmd) = at_and_ucmd!();
+    let name = "test_split_bytes_prime_part_size";
+    RandomFile::new(&at, name).add_bytes(10000);
+    // 1753 is prime and greater than the buffer size, 1024.
+    ucmd.args(&["-b", "1753", name, "b"]).succeeds();
+
+    let glob = Glob::new(&at, ".", r"b[[:alpha:]][[:alpha:]]$");
+    assert_eq!(glob.count(), 6);
+    let mut fns = glob.collect();
+    // glob.collect() is not guaranteed to return in sorted order, so we sort.
+    fns.sort();
+    for i in 0..5 {
+        assert_eq!(glob.directory.metadata(&fns[i]).len(), 1753);
+    }
+    assert_eq!(glob.directory.metadata(&fns[5]).len(), 1235);
+    assert_eq!(glob.collate(), at.read_bytes(name));
 }
 
 #[test]
 fn test_split_num_prefixed_chunks_by_lines() {
     let (at, mut ucmd) = at_and_ucmd!();
     let name = "split_num_prefixed_chunks_by_lines";
-    let glob = Glob::new(&at, ".", r"c\d\d$");
     RandomFile::new(&at, name).add_lines(10000);
     ucmd.args(&["-d", "-l", "1000", name, "c"]).succeeds();
+
+    let glob = Glob::new(&at, ".", r"c\d\d$");
     assert_eq!(glob.count(), 10);
-    assert_eq!(glob.collate(), at.read(name).into_bytes());
+    assert_eq!(glob.collate(), at.read_bytes(name));
 }
 
 #[test]
 fn test_split_str_prefixed_chunks_by_lines() {
     let (at, mut ucmd) = at_and_ucmd!();
     let name = "split_str_prefixed_chunks_by_lines";
-    let glob = Glob::new(&at, ".", r"d[[:alpha:]][[:alpha:]]$");
     RandomFile::new(&at, name).add_lines(10000);
     ucmd.args(&["-l", "1000", name, "d"]).succeeds();
+
+    let glob = Glob::new(&at, ".", r"d[[:alpha:]][[:alpha:]]$");
     assert_eq!(glob.count(), 10);
-    assert_eq!(glob.collate(), at.read(name).into_bytes());
+    assert_eq!(glob.collate(), at.read_bytes(name));
 }
 
 #[test]
 fn test_split_additional_suffix() {
     let (at, mut ucmd) = at_and_ucmd!();
     let name = "split_additional_suffix";
-    let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]].txt$");
     RandomFile::new(&at, name).add_lines(2000);
     ucmd.args(&["--additional-suffix", ".txt", name]).succeeds();
+
+    let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]].txt$");
     assert_eq!(glob.count(), 2);
-    assert_eq!(glob.collate(), at.read(name).into_bytes());
+    assert_eq!(glob.collate(), at.read_bytes(name));
 }
 
 // note: the test_filter* tests below are unix-only
@@ -182,15 +233,16 @@ fn test_filter() {
     // like `test_split_default()` but run a command before writing
     let (at, mut ucmd) = at_and_ucmd!();
     let name = "filtered";
-    let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$");
     let n_lines = 3;
     RandomFile::new(&at, name).add_lines(n_lines);
 
     // change all characters to 'i'
     ucmd.args(&["--filter=sed s/./i/g > $FILE", name])
         .succeeds();
+
     // assert all characters are 'i' / no character is not 'i'
     // (assert that command succeded)
+    let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$");
     assert!(
         glob.collate().iter().find(|&&c| {
             // is not i
@@ -209,7 +261,6 @@ fn test_filter_with_env_var_set() {
     // implemented like `test_split_default()` but run a command before writing
     let (at, mut ucmd) = at_and_ucmd!();
     let name = "filtered";
-    let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$");
     let n_lines = 3;
     RandomFile::new(&at, name).add_lines(n_lines);
 
@@ -217,7 +268,9 @@ fn test_filter_with_env_var_set() {
     env::set_var("FILE", &env_var_value);
     ucmd.args(&[format!("--filter={}", "cat > $FILE").as_str(), name])
         .succeeds();
-    assert_eq!(glob.collate(), at.read(name).into_bytes());
+
+    let glob = Glob::new(&at, ".", r"x[[:alpha:]][[:alpha:]]$");
+    assert_eq!(glob.collate(), at.read_bytes(name));
     assert!(env::var("FILE").unwrap_or("var was unset".to_owned()) == env_var_value);
 }
 

tests/by-util/test_stat.rs

@@ -9,42 +9,6 @@ pub use self::stat::*;
 mod test_fsext {
     use super::*;
 
-    #[test]
-    fn test_access() {
-        assert_eq!("drwxr-xr-x", pretty_access(S_IFDIR | 0o755));
-        assert_eq!("-rw-r--r--", pretty_access(S_IFREG | 0o644));
-        assert_eq!("srw-r-----", pretty_access(S_IFSOCK | 0o640));
-        assert_eq!("lrw-r-xr-x", pretty_access(S_IFLNK | 0o655));
-        assert_eq!("?rw-r-xr-x", pretty_access(0o655));
-
-        assert_eq!(
-            "brwSr-xr-x",
-            pretty_access(S_IFBLK | S_ISUID as mode_t | 0o655)
-        );
-        assert_eq!(
-            "brwsr-xr-x",
-            pretty_access(S_IFBLK | S_ISUID as mode_t | 0o755)
-        );
-
-        assert_eq!(
-            "prw---sr--",
-            pretty_access(S_IFIFO | S_ISGID as mode_t | 0o614)
-        );
-        assert_eq!(
-            "prw---Sr--",
-            pretty_access(S_IFIFO | S_ISGID as mode_t | 0o604)
-        );
-
-        assert_eq!(
-            "c---r-xr-t",
-            pretty_access(S_IFCHR | S_ISVTX as mode_t | 0o055)
-        );
-        assert_eq!(
-            "c---r-xr-T",
-            pretty_access(S_IFCHR | S_ISVTX as mode_t | 0o054)
-        );
-    }
-
     #[test]
     fn test_file_type() {
         assert_eq!("block special file", pretty_filetype(S_IFBLK, 0));
@@ -198,9 +162,16 @@ fn test_terse_normal_format() {
     let expect = expected_result(&args);
     println!("actual: {:?}", actual);
     println!("expect: {:?}", expect);
-    let v_actual: Vec<&str> = actual.split(' ').collect();
+    let v_actual: Vec<&str> = actual.trim().split(' ').collect();
-    let v_expect: Vec<&str> = expect.split(' ').collect();
+    let mut v_expect: Vec<&str> = expect.trim().split(' ').collect();
     assert!(!v_expect.is_empty());
+
+    // uu_stat does not support selinux
+    if v_actual.len() == v_expect.len() - 1 && v_expect[v_expect.len() - 1].contains(":") {
+        // assume last element contains: `SELinux security context string`
+        v_expect.pop();
+    }
+
     // * allow for inequality if `stat` (aka, expect) returns "0" (unknown value)
     assert!(
         expect == "0"

tests/by-util/test_wc.rs

@@ -112,3 +112,50 @@ fn test_multiple_default() {
              alice_in_wonderland.txt\n  36  370 2189 total\n",
         );
 }
+
+/// Test for an empty file.
+#[test]
+fn test_file_empty() {
+    new_ucmd!()
+        .args(&["-clmwL", "emptyfile.txt"])
+        .run()
+        .stdout_is("0 0 0 0 0 emptyfile.txt\n");
+}
+
+/// Test for an file containing a single non-whitespace character
+/// *without* a trailing newline.
+#[test]
+fn test_file_single_line_no_trailing_newline() {
+    new_ucmd!()
+        .args(&["-clmwL", "notrailingnewline.txt"])
+        .run()
+        .stdout_is("1 1 2 2 1 notrailingnewline.txt\n");
+}
+
+/// Test for a file that has 100 empty lines (that is, the contents of
+/// the file are the newline character repeated one hundred times).
+#[test]
+fn test_file_many_empty_lines() {
+    new_ucmd!()
+        .args(&["-clmwL", "manyemptylines.txt"])
+        .run()
+        .stdout_is("100   0 100 100   0 manyemptylines.txt\n");
+}
+
+/// Test for a file that has one long line comprising only spaces.
+#[test]
+fn test_file_one_long_line_only_spaces() {
+    new_ucmd!()
+        .args(&["-clmwL", "onelongemptyline.txt"])
+        .run()
+        .stdout_is("    1     0 10001 10001 10000 onelongemptyline.txt\n");
+}
+
+/// Test for a file that has one long line comprising a single "word".
+#[test]
+fn test_file_one_long_word() {
+    new_ucmd!()
+        .args(&["-clmwL", "onelongword.txt"])
+        .run()
+        .stdout_is("    1     1 10001 10001 10000 onelongword.txt\n");
+}

tests/by-util/test_who.rs

@@ -1,11 +1,13 @@
-#[cfg(target_os = "linux")]
 use crate::common::util::*;
 
 #[cfg(target_os = "linux")]
 #[test]
 fn test_count() {
     for opt in vec!["-q", "--count"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
@@ -13,17 +15,21 @@ fn test_count() {
 #[test]
 fn test_boot() {
     for opt in vec!["-b", "--boot"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
 #[cfg(target_os = "linux")]
 #[test]
 fn test_heading() {
-    for opt in vec!["-H"] {
+    for opt in vec!["-H", "--heading"] {
         // allow whitespace variation
-        // * minor whitespace differences occur between platform built-in outputs; specifically number of TABs between "TIME" and "COMMENT" may be variant
+        // * minor whitespace differences occur between platform built-in outputs;
-        let actual = new_ucmd!().arg(opt).run().stdout_move_str();
+        //   specifically number of TABs between "TIME" and "COMMENT" may be variant
+        let actual = new_ucmd!().arg(opt).succeeds().stdout_move_str();
         let expect = expected_result(opt);
         println!("actual: {:?}", actual);
         println!("expect: {:?}", expect);
@@ -37,7 +43,10 @@ fn test_heading() {
 #[test]
 fn test_short() {
     for opt in vec!["-s", "--short"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
@@ -45,7 +54,10 @@ fn test_short() {
 #[test]
 fn test_login() {
     for opt in vec!["-l", "--login"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
@@ -53,7 +65,110 @@ fn test_login() {
 #[test]
 fn test_m() {
     for opt in vec!["-m"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_process() {
+    for opt in vec!["-p", "--process"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_runlevel() {
+    for opt in vec!["-r", "--runlevel"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_time() {
+    for opt in vec!["-t", "--time"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_mesg() {
+    for opt in vec!["-w", "-T", "--users", "--message", "--writable"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_arg1_arg2() {
+    let scene = TestScenario::new(util_name!());
+
+    let expected = scene
+        .cmd_keepenv(util_name!())
+        .env("LANGUAGE", "C")
+        .arg("am")
+        .arg("i")
+        .succeeds();
+
+    scene
+        .ucmd()
+        .arg("am")
+        .arg("i")
+        .succeeds()
+        .stdout_is(expected.stdout_str());
+}
+
+#[test]
+fn test_too_many_args() {
+    let expected =
+        "error: The value 'u' was provided to '<FILE>...', but it wasn't expecting any more values";
+
+    new_ucmd!()
+        .arg("am")
+        .arg("i")
+        .arg("u")
+        .fails()
+        .stderr_contains(expected);
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+fn test_users() {
+    for opt in vec!["-u", "--users"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
+    }
+}
+
+#[cfg(target_os = "linux")]
+#[test]
+#[ignore]
+fn test_lookup() {
+    for opt in vec!["--lookup"] {
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
@@ -61,15 +176,60 @@ fn test_m() {
 #[test]
 fn test_dead() {
     for opt in vec!["-d", "--dead"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
+#[cfg(target_os = "linux")]
+#[test]
+fn test_all_separately() {
+    // -a, --all         same as -b -d --login -p -r -t -T -u
+    let scene = TestScenario::new(util_name!());
+
+    let expected = scene
+        .cmd_keepenv(util_name!())
+        .env("LANGUAGE", "C")
+        .arg("-b")
+        .arg("-d")
+        .arg("--login")
+        .arg("-p")
+        .arg("-r")
+        .arg("-t")
+        .arg("-T")
+        .arg("-u")
+        .succeeds();
+
+    scene
+        .ucmd()
+        .arg("-b")
+        .arg("-d")
+        .arg("--login")
+        .arg("-p")
+        .arg("-r")
+        .arg("-t")
+        .arg("-T")
+        .arg("-u")
+        .succeeds()
+        .stdout_is(expected.stdout_str());
+
+    scene
+        .ucmd()
+        .arg("--all")
+        .succeeds()
+        .stdout_is(expected.stdout_str());
+}
+
 #[cfg(target_os = "linux")]
 #[test]
 fn test_all() {
     for opt in vec!["-a", "--all"] {
-        new_ucmd!().arg(opt).run().stdout_is(expected_result(opt));
+        new_ucmd!()
+            .arg(opt)
+            .succeeds()
+            .stdout_is(expected_result(opt));
     }
 }
 
@@ -79,6 +239,6 @@ fn expected_result(arg: &str) -> String {
         .cmd_keepenv(util_name!())
         .env("LANGUAGE", "C")
         .args(&[arg])
-        .run()
+        .succeeds()
         .stdout_move_str()
 }

tests/common/util.rs

@@ -163,7 +163,7 @@ impl CmdResult {
 
     /// asserts that the command's exit code is the same as the given one
     pub fn status_code(&self, code: i32) -> &CmdResult {
-        assert!(self.code == Some(code));
+        assert_eq!(self.code, Some(code));
         self
     }
 
@@ -295,12 +295,22 @@ impl CmdResult {
     }
 
     pub fn stdout_contains<T: AsRef<str>>(&self, cmp: T) -> &CmdResult {
-        assert!(self.stdout_str().contains(cmp.as_ref()));
+        assert!(
+            self.stdout_str().contains(cmp.as_ref()),
+            "'{}' does not contain '{}'",
+            self.stdout_str(),
+            cmp.as_ref()
+        );
         self
     }
 
     pub fn stderr_contains<T: AsRef<str>>(&self, cmp: T) -> &CmdResult {
-        assert!(self.stderr_str().contains(cmp.as_ref()));
+        assert!(
+            self.stderr_str().contains(cmp.as_ref()),
+            "'{}' does not contain '{}'",
+            self.stderr_str(),
+            cmp.as_ref()
+        );
         self
     }
 

tests/fixtures/wc/manyemptylines.txt

@@ -0,0 +1,100 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+

tests/fixtures/wc/notrailingnewline.txt

@@ -0,0 +1 @@
+a