From fe3645d4d5c34ef830ef04edc5f5fe50edcdc472 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Fri, 10 Sep 2021 21:33:34 +0200 Subject: [PATCH 01/47] ls: add support for showing SELinux context (--context/-Z) --- Cargo.lock | 1 + Cargo.toml | 2 +- src/uu/ls/Cargo.toml | 4 ++ src/uu/ls/src/ls.rs | 164 +++++++++++++++++++++++++++++++++++++------ 4 files changed, 149 insertions(+), 22 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 704d1eea1..ea3467b5a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2632,6 +2632,7 @@ dependencies = [ "lscolors", "number_prefix", "once_cell", + "selinux", "term_grid", "termsize", "unicode-width", diff --git a/Cargo.toml b/Cargo.toml index 3a2c5f12a..8d2f2da58 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -147,7 +147,7 @@ feat_os_unix_musl = [ # NOTE: # The selinux(-sys) crate requires `libselinux` headers and shared library to be accessible in the C toolchain at compile time. # Running a uutils compiled with `feat_selinux` requires an SELinux enabled Kernel at run time. -feat_selinux = ["cp/selinux", "id/selinux", "selinux", "feat_require_selinux"] +feat_selinux = ["cp/selinux", "id/selinux", "ls/selinux", "selinux", "feat_require_selinux"] # "feat_acl" == set of utilities providing support for acl (access control lists) if enabled with `--features feat_acl`. # NOTE: # On linux, the posix-acl/acl-sys crate requires `libacl` headers and shared library to be accessible in the C toolchain at compile time. diff --git a/src/uu/ls/Cargo.toml b/src/uu/ls/Cargo.toml index dbe6bacaa..62dcd59a4 100644 --- a/src/uu/ls/Cargo.toml +++ b/src/uu/ls/Cargo.toml @@ -28,6 +28,7 @@ uucore = { version = ">=0.0.8", package = "uucore", path = "../../uucore", featu uucore_procs = { version=">=0.0.6", package = "uucore_procs", path = "../../uucore_procs" } once_cell = "1.7.2" atty = "0.2" +selinux = { version="0.2.1", optional = true } [target.'cfg(unix)'.dependencies] lazy_static = "1.4.0" @@ -35,3 +36,6 @@ lazy_static = "1.4.0" [[bin]] name = "ls" path = "src/main.rs" + +[features] +feat_selinux = ["selinux"] diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 6f63c2a4a..7b7fd873e 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -47,6 +47,11 @@ use unicode_width::UnicodeWidthStr; use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR}; use uucore::{fs::display_permissions, version_cmp::version_cmp}; +#[cfg(not(feature = "selinux"))] +static CONTEXT_HELP_TEXT: &str = "print any security context of each file (not enabled)"; +#[cfg(feature = "selinux")] +static CONTEXT_HELP_TEXT: &str = "print any security context of each file"; + fn usage() -> String { format!("{0} [OPTION]... [FILE]...", uucore::execution_phrase()) } @@ -126,6 +131,7 @@ pub mod options { pub static FULL_TIME: &str = "full-time"; pub static HIDE: &str = "hide"; pub static IGNORE: &str = "ignore"; + pub static CONTEXT: &str = "context"; } const DEFAULT_TERM_WIDTH: u16 = 80; @@ -236,6 +242,8 @@ struct Config { quoting_style: QuotingStyle, indicator_style: IndicatorStyle, time_style: TimeStyle, + context: bool, + selinux_supported: bool, } // Fields that can be removed or added to the long format @@ -247,9 +255,18 @@ struct LongFormat { numeric_uid_gid: bool, } +struct PaddingCollection { + longest_link_count_len: usize, + longest_uname_len: usize, + longest_group_len: usize, + longest_context_len: usize, + longest_size_len: usize, +} + impl Config { #[allow(clippy::cognitive_complexity)] fn from(options: &clap::ArgMatches) -> UResult { + let context = options.is_present(options::CONTEXT); let (mut format, opt) = if let Some(format_) = options.value_of(options::FORMAT) { ( match format_ { @@ -593,6 +610,17 @@ impl Config { quoting_style, indicator_style, time_style, + context, + selinux_supported: { + #[cfg(feature = "selinux")] + { + selinux::kernel_support() != selinux::KernelSupport::Unsupported + } + #[cfg(not(feature = "selinux"))] + { + false + } + }, }) } } @@ -611,6 +639,10 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(|v| v.map(Path::new).collect()) .unwrap_or_else(|| vec![Path::new(".")]); + if config.context && !config.selinux_supported { + show_warning!("--context (-Z) works only on an SELinux-enabled kernel",); + } + list(locs, config) } @@ -1154,6 +1186,12 @@ only ignore '.' and '..'.", .overrides_with(options::FULL_TIME) .help("like -l --time-style=full-iso"), ) + .arg( + Arg::with_name(options::CONTEXT) + .short("Z") + .long(options::CONTEXT) + .help(CONTEXT_HELP_TEXT), + ) // Positional arguments .arg( Arg::with_name(options::PATHS) @@ -1178,6 +1216,7 @@ struct PathData { // PathBuf that all above data corresponds to p_buf: PathBuf, must_dereference: bool, + security_context: String, } impl PathData { @@ -1221,12 +1260,39 @@ impl PathData { None => OnceCell::new(), }; + let security_context: String = if config.context { + if config.selinux_supported { + #[cfg(feature = "selinux")] + { + if let Ok(Some(context)) = + selinux::SecurityContext::of_path(&p_buf, must_dereference, false) + { + String::from_utf8_lossy(context.as_bytes()) + .trim_end_matches(char::from(0)) + .to_string() + } else { + // TODO: print warning with error to stderr + "?".to_string() + } + } + #[cfg(not(feature = "selinux"))] + { + "?".to_string() + } + } else { + "?".to_string() + } + } else { + String::new() + }; + Self { md: OnceCell::new(), ft, display_name, p_buf, must_dereference, + security_context, } } @@ -1390,7 +1456,7 @@ fn get_metadata(entry: &Path, dereference: bool) -> std::io::Result { } fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize, usize, usize) { - // TODO: Cache/memoize the display_* results so we don't have to recalculate them. + // TODO: Cache/memorize the display_* results so we don't have to recalculate them. if let Some(md) = entry.md() { ( display_symlink_count(md).len(), @@ -1403,31 +1469,40 @@ fn display_dir_entry_size(entry: &PathData, config: &Config) -> (usize, usize, u } } -fn pad_left(string: String, count: usize) -> String { +fn pad_left(string: &str, count: usize) -> String { format!("{:>width$}", string, width = count) } -fn pad_right(string: String, count: usize) -> String { +fn pad_right(string: &str, count: usize) -> String { format!("{:) { + // `-Z`, `--context`: + // Display the SELinux security context or '?' if none is found. When used with the `-l` + // option, print the security context to the left of the size column. + if config.format == Format::Long { let ( mut longest_link_count_len, mut longest_uname_len, mut longest_group_len, + mut longest_context_len, mut longest_size_len, - ) = (1, 1, 1, 1); + ) = (1, 1, 1, 1, 1); let mut total_size = 0; for item in items { + let context_len = item.security_context.len(); let (link_count_len, uname_len, group_len, size_len) = display_dir_entry_size(item, config); longest_link_count_len = link_count_len.max(longest_link_count_len); longest_size_len = size_len.max(longest_size_len); longest_uname_len = uname_len.max(longest_uname_len); longest_group_len = group_len.max(longest_group_len); + if config.context { + longest_context_len = context_len.max(longest_context_len); + } longest_size_len = size_len.max(longest_size_len); total_size += item.md().map_or(0, |md| get_block_size(md, config)); } @@ -1439,16 +1514,31 @@ fn display_items(items: &[PathData], config: &Config, out: &mut BufWriter display_grid(names, config.width, Direction::TopToBottom, out), @@ -1573,15 +1663,13 @@ fn display_grid( /// longest_link_count_len: usize, /// longest_uname_len: usize, /// longest_group_len: usize, +/// longest_context_len: usize, /// longest_size_len: usize, /// ``` /// that decide the maximum possible character count of each field. fn display_item_long( item: &PathData, - longest_link_count_len: usize, - longest_uname_len: usize, - longest_group_len: usize, - longest_size_len: usize, + padding: PaddingCollection, config: &Config, out: &mut BufWriter, ) { @@ -1602,16 +1690,23 @@ fn display_item_long( let _ = write!( out, - "{} {}", + "{}{} {}", display_permissions(md, true), - pad_left(display_symlink_count(md), longest_link_count_len), + if item.security_context != "?" { + // GNU `ls` uses a "." character to indicate a file with a security context, but not + // other alternate access method. + "." + } else { + "" + }, + pad_left(&display_symlink_count(md), padding.longest_link_count_len), ); if config.long.owner { let _ = write!( out, " {}", - pad_right(display_uname(md, config), longest_uname_len) + pad_right(&display_uname(md, config), padding.longest_uname_len) ); } @@ -1619,7 +1714,15 @@ fn display_item_long( let _ = write!( out, " {}", - pad_right(display_group(md, config), longest_group_len) + pad_right(&display_group(md, config), padding.longest_group_len) + ); + } + + if config.context { + let _ = write!( + out, + " {}", + pad_right(&item.security_context, padding.longest_context_len) ); } @@ -1629,19 +1732,19 @@ fn display_item_long( let _ = write!( out, " {}", - pad_right(display_uname(md, config), longest_uname_len) + pad_right(&display_uname(md, config), padding.longest_uname_len) ); } let _ = writeln!( out, " {} {} {}", - pad_left(display_size_or_rdev(md, config), longest_size_len), + pad_left(&display_size_or_rdev(md, config), padding.longest_size_len), 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 // start of the function. - display_file_name(item, config).unwrap().contents, + display_file_name(item, config, None).unwrap().contents, ); } @@ -1865,10 +1968,15 @@ fn classify_file(path: &PathData) -> Option { /// * `config.indicator_style` to append specific characters to `name` using [`classify_file`]. /// * `config.format` to display symlink targets if `Format::Long`. This function is also /// responsible for coloring symlink target names if `config.color` is specified. +/// * `config.context` to prepend security context to `name` if compiled with `feat_selinux`. /// /// Note that non-unicode sequences in symlink targets are dealt with using /// [`std::path::Path::to_string_lossy`]. -fn display_file_name(path: &PathData, config: &Config) -> Option { +fn display_file_name( + path: &PathData, + config: &Config, + prefix_context: Option, +) -> Option { // This is our return value. We start by `&path.display_name` and modify it along the way. let mut name = escape_name(&path.display_name, &config.quoting_style); @@ -1960,6 +2068,20 @@ fn display_file_name(path: &PathData, config: &Config) -> Option { } } + // Prepend the security context to the `name` and adjust `width` in order + // to get correct alignment from later calls to`display_grid()`. + if config.context { + if let Some(pad_count) = prefix_context { + let security_context = if !matches!(config.format, Format::Commas) { + pad_left(&path.security_context, pad_count) + } else { + path.security_context.to_owned() + }; + name = format!("{} {}", security_context, name); + width += security_context.len() + 1; + } + } + Some(Cell { contents: name, width, From e018a45b54b96e1e3f8dd323b411e0bc6ae84673 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Fri, 10 Sep 2021 21:35:00 +0200 Subject: [PATCH 02/47] test_ls: add/adjust tests for SELinux context --- src/uu/ls/src/ls.rs | 12 ++-- tests/by-util/test_ls.rs | 127 +++++++++++++++++++++++++++------------ 2 files changed, 94 insertions(+), 45 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 7b7fd873e..5c26c8e26 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -639,10 +639,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { .map(|v| v.map(Path::new).collect()) .unwrap_or_else(|| vec![Path::new(".")]); - if config.context && !config.selinux_supported { - show_warning!("--context (-Z) works only on an SELinux-enabled kernel",); - } - list(locs, config) } @@ -1260,7 +1256,7 @@ impl PathData { None => OnceCell::new(), }; - let security_context: String = if config.context { + let security_context = if config.context { if config.selinux_supported { #[cfg(feature = "selinux")] { @@ -1692,9 +1688,9 @@ fn display_item_long( out, "{}{} {}", display_permissions(md, true), - if item.security_context != "?" { - // GNU `ls` uses a "." character to indicate a file with a security context, but not - // other alternate access method. + if item.security_context.len() > 1 { + // GNU `ls` uses a "." character to indicate a file with a security context, + // but not other alternate access method. "." } else { "" diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 3d6092416..f14fc424a 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -347,6 +347,7 @@ fn test_ls_long_format() { // A line of the output should be: // One of the characters -bcCdDlMnpPsStTx? // rwx, with - for missing permissions, thrice. + // Zero or one "." for indicating a file with security context // A number, preceded by column whitespace, and followed by a single space. // A username, currently [^ ], followed by column whitespace, twice (or thrice for Hurd). // A number, followed by a single space. @@ -356,13 +357,13 @@ fn test_ls_long_format() { // and followed by a single space. // Whatever comes after is irrelevant to this specific test. scene.ucmd().arg(arg).arg("test-long-dir").succeeds().stdout_matches(&Regex::new( - r"\n[-bcCdDlMnpPsStTx?]([r-][w-][xt-]){3} +\d+ [^ ]+ +[^ ]+( +[^ ]+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ " + r"\n[-bcCdDlMnpPsStTx?]([r-][w-][xt-]){3}\.? +\d+ [^ ]+ +[^ ]+( +[^ ]+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ " ).unwrap()); } // This checks for the line with the .. entry. The uname and group should be digits. scene.ucmd().arg("-lan").arg("test-long-dir").succeeds().stdout_matches(&Regex::new( - r"\nd([r-][w-][xt-]){3} +\d+ \d+ +\d+( +\d+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ \.\." + r"\nd([r-][w-][xt-]){3}\.? +\d+ \d+ +\d+( +\d+)? +\d+ [A-Z][a-z]{2} {0,2}\d{0,2} {0,2}[0-9:]+ \.\." ).unwrap()); } @@ -370,6 +371,7 @@ fn test_ls_long_format() { /// This test is mainly about coloring, but, the recursion, symlink `->` processing, /// and `.` and `..` being present in `-a` all need to work for the test to pass. /// This test does not really test anything provided by `-l` but the file names and symlinks. +#[cfg(all(feature = "ln", feature = "mkdir", feature = "touch"))] #[test] fn test_ls_long_symlink_color() { // If you break this test after breaking mkdir, touch, or ln, do not be alarmed! @@ -639,55 +641,57 @@ fn test_ls_long_formats() { let at = &scene.fixtures; at.touch(&at.plus_as_string("test-long-formats")); + // Zero or one "." for indicating a file with security context + // Regex for three names, so all of author, group and owner - let re_three = Regex::new(r"[xrw-]{9} \d ([-0-9_a-z]+ ){3}0").unwrap(); + let re_three = Regex::new(r"[xrw-]{9}\.? \d ([-0-9_a-z]+ ){3}0").unwrap(); #[cfg(unix)] - let re_three_num = Regex::new(r"[xrw-]{9} \d (\d+ ){3}0").unwrap(); + let re_three_num = Regex::new(r"[xrw-]{9}\.? \d (\d+ ){3}0").unwrap(); // Regex for two names, either: // - group and owner // - author and owner // - author and group - let re_two = Regex::new(r"[xrw-]{9} \d ([-0-9_a-z]+ ){2}0").unwrap(); + let re_two = Regex::new(r"[xrw-]{9}\.? \d ([-0-9_a-z]+ ){2}0").unwrap(); #[cfg(unix)] - let re_two_num = Regex::new(r"[xrw-]{9} \d (\d+ ){2}0").unwrap(); + let re_two_num = Regex::new(r"[xrw-]{9}\.? \d (\d+ ){2}0").unwrap(); // Regex for one name: author, group or owner - let re_one = Regex::new(r"[xrw-]{9} \d [-0-9_a-z]+ 0").unwrap(); + let re_one = Regex::new(r"[xrw-]{9}\.? \d [-0-9_a-z]+ 0").unwrap(); #[cfg(unix)] - let re_one_num = Regex::new(r"[xrw-]{9} \d \d+ 0").unwrap(); + let re_one_num = Regex::new(r"[xrw-]{9}\.? \d \d+ 0").unwrap(); // Regex for no names - let re_zero = Regex::new(r"[xrw-]{9} \d 0").unwrap(); + let re_zero = Regex::new(r"[xrw-]{9}\.? \d 0").unwrap(); - let result = scene + scene .ucmd() .arg("-l") .arg("--author") .arg("test-long-formats") - .succeeds(); - assert!(re_three.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_three); - let result = scene + scene .ucmd() .arg("-l1") .arg("--author") .arg("test-long-formats") - .succeeds(); - assert!(re_three.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_three); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .arg("--author") .arg("test-long-formats") - .succeeds(); - assert!(re_three_num.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_three_num); } for arg in &[ @@ -697,22 +701,22 @@ fn test_ls_long_formats() { "-lG --author", // only author and owner "-l --no-group --author", // only author and owner ] { - let result = scene + scene .ucmd() .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_two.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_two); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_two_num.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_two_num); } } @@ -726,22 +730,22 @@ fn test_ls_long_formats() { "-l --no-group", // only owner "-gG --author", // only author ] { - let result = scene + scene .ucmd() .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_one.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_one); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_one_num.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_one_num); } } @@ -758,22 +762,22 @@ fn test_ls_long_formats() { "-og1", "-og1l", ] { - let result = scene + scene .ucmd() .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_zero.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_zero); #[cfg(unix)] { - let result = scene + scene .ucmd() .arg("-n") .args(&arg.split(' ').collect::>()) .arg("test-long-formats") - .succeeds(); - assert!(re_zero.is_match(result.stdout_str())); + .succeeds() + .stdout_matches(&re_zero); } } } @@ -1251,7 +1255,7 @@ fn test_ls_inode() { at.touch(file); let re_short = Regex::new(r" *(\d+) test_inode").unwrap(); - let re_long = Regex::new(r" *(\d+) [xrw-]{10} \d .+ test_inode").unwrap(); + let re_long = Regex::new(r" *(\d+) [xrw-]{10}\.? \d .+ test_inode").unwrap(); let result = scene.ucmd().arg("test_inode").arg("-i").succeeds(); assert!(re_short.is_match(result.stdout_str())); @@ -2275,3 +2279,52 @@ fn test_ls_dangling_symlinks() { .succeeds() // this should fail, though at the moment, ls lacks a way to propagate errors encountered during display .stdout_contains(if cfg!(windows) { "dangle" } else { "? dangle" }); } + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_ls_context() { + use selinux::{self, KernelSupport}; + if selinux::kernel_support() == KernelSupport::Unsupported { + println!("test skipped: Kernel has no support for SElinux context",); + return; + } + let ts = TestScenario::new(util_name!()); + for c_flag in &["-Z", "--context"] { + ts.ucmd() + .args(&[c_flag, &"/"]) + .succeeds() + .stdout_only(unwrap_or_return!(expected_result(&ts, &[c_flag, &"/"])).stdout_str()); + } +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_ls_context_format() { + use selinux::{self, KernelSupport}; + if selinux::kernel_support() == KernelSupport::Unsupported { + println!("test skipped: Kernel has no support for SElinux context",); + return; + } + let ts = TestScenario::new(util_name!()); + // NOTE: + // --format=long/verbose matches the output of GNU's ls for --context + // except for the size count which may differ to the size count reported by GNU's ls. + for word in &[ + "across", + "commas", + "horizontal", + // "long", + "single-column", + // "verbose", + "vertical", + ] { + let format = format!("--format={}", word); + ts.ucmd() + .args(&[&"-Z", &format.as_str(), &"/"]) + .succeeds() + .stdout_only( + unwrap_or_return!(expected_result(&ts, &[&"-Z", &format.as_str(), &"/"])) + .stdout_str(), + ); + } +} From 7e577476d81f36cac9f3dd64df924b3761e53b49 Mon Sep 17 00:00:00 2001 From: Smicry Date: Sun, 12 Sep 2021 23:20:05 +0800 Subject: [PATCH 03/47] add kill -l final new line #2644 --- src/uu/kill/src/kill.rs | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/uu/kill/src/kill.rs b/src/uu/kill/src/kill.rs index 494dc0602..b1cbba17a 100644 --- a/src/uu/kill/src/kill.rs +++ b/src/uu/kill/src/kill.rs @@ -160,18 +160,13 @@ fn print_signal(signal_name_or_value: &str) -> UResult<()> { } fn print_signals() { - let mut pos = 0; for (idx, signal) in ALL_SIGNALS.iter().enumerate() { - pos += signal.len(); - print!("{}", signal); - if idx > 0 && pos > 73 { - println!(); - pos = 0; - } else { - pos += 1; + if idx > 0 { print!(" "); } + print!("{}", signal); } + println!(); } fn list(arg: Option) -> UResult<()> { From 7d445612a29a8a2afba61c4024923be13253aca8 Mon Sep 17 00:00:00 2001 From: Smicry Date: Mon, 13 Sep 2021 00:15:53 +0800 Subject: [PATCH 04/47] add kill list final new line test --- tests/by-util/test_kill.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/tests/by-util/test_kill.rs b/tests/by-util/test_kill.rs index fe5d4557a..9afac2862 100644 --- a/tests/by-util/test_kill.rs +++ b/tests/by-util/test_kill.rs @@ -56,6 +56,14 @@ fn test_kill_list_all_signals() { .stdout_contains("HUP"); } +#[test] +fn test_kill_list_final_new_line() { + new_ucmd!() + .arg("-l") + .succeeds() + .stdout_matches(&Regex::new("\\n$").unwrap()); +} + #[test] fn test_kill_list_all_signals_as_table() { // Check for a few signals. Do not try to be comprehensive. From db21fef95de9931a2d0c69cf0ccf3b18966f013d Mon Sep 17 00:00:00 2001 From: Smicry Date: Mon, 13 Sep 2021 09:27:14 +0800 Subject: [PATCH 05/47] fix kill list final new line test --- tests/by-util/test_kill.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/by-util/test_kill.rs b/tests/by-util/test_kill.rs index 9afac2862..a16760689 100644 --- a/tests/by-util/test_kill.rs +++ b/tests/by-util/test_kill.rs @@ -61,7 +61,7 @@ fn test_kill_list_final_new_line() { new_ucmd!() .arg("-l") .succeeds() - .stdout_matches(&Regex::new("\\n$").unwrap()); + .stdout_matches(&Regex::new("[\\n\\r]$").unwrap()); } #[test] From cd2153eac624232ff240b323eb4c04c110308e53 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Mon, 13 Sep 2021 13:24:44 +0200 Subject: [PATCH 06/47] ls: (--context/-Z) add support for invalid utf8 If security context byte slice contains invalid utf8: * issue warning * show replacement characters for invalid utf8 sequences --- src/uu/ls/src/ls.rs | 37 ++++++++++++++++++++++++++----------- tests/by-util/test_ls.rs | 18 +++++++++++++++++- 2 files changed, 43 insertions(+), 12 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 5c26c8e26..be1964e43 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -40,6 +40,7 @@ use std::{ time::Duration, }; use term_grid::{Cell, Direction, Filling, Grid, GridOptions}; +use uucore::display::Quotable; use uucore::error::{set_exit_code, FromIo, UError, UResult}; use unicode_width::UnicodeWidthStr; @@ -1256,27 +1257,41 @@ impl PathData { None => OnceCell::new(), }; + let substitute_string = "?".to_string(); let security_context = if config.context { if config.selinux_supported { #[cfg(feature = "selinux")] { - if let Ok(Some(context)) = - selinux::SecurityContext::of_path(&p_buf, must_dereference, false) - { - String::from_utf8_lossy(context.as_bytes()) - .trim_end_matches(char::from(0)) - .to_string() - } else { - // TODO: print warning with error to stderr - "?".to_string() + match selinux::SecurityContext::of_path(&p_buf, must_dereference, false) { + Err(_r) => { + // TODO: show the actual reason why it failed + show_warning!("failed to get security context of: {}", p_buf.quote()); + substitute_string + } + Ok(None) => substitute_string, + Ok(Some(context)) => { + let mut context = context.as_bytes(); + if context.ends_with(&[0]) { + // TODO: replace with `strip_prefix()` when MSRV >= 1.51 + context = &context[..context.len() - 1] + }; + String::from_utf8(context.to_vec()).unwrap_or_else(|e| { + show_warning!( + "getting security context of: {}: {}", + p_buf.quote(), + e.to_string() + ); + String::from_utf8_lossy(context).into_owned() + }) + } } } #[cfg(not(feature = "selinux"))] { - "?".to_string() + substitute_string } } else { - "?".to_string() + substitute_string } } else { String::new() diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index f14fc424a..58119aac3 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -2282,7 +2282,23 @@ fn test_ls_dangling_symlinks() { #[test] #[cfg(feature = "feat_selinux")] -fn test_ls_context() { +fn test_ls_context1() { + use selinux::{self, KernelSupport}; + if selinux::kernel_support() == KernelSupport::Unsupported { + println!("test skipped: Kernel has no support for SElinux context",); + return; + } + + let file = "test_ls_context_file"; + let expected = format!("unconfined_u:object_r:user_tmp_t:s0 {}\n", file); + let (at, mut ucmd) = at_and_ucmd!(); + at.touch(file); + ucmd.args(&["-Z", file]).succeeds().stdout_is(expected); +} + +#[test] +#[cfg(feature = "feat_selinux")] +fn test_ls_context2() { use selinux::{self, KernelSupport}; if selinux::kernel_support() == KernelSupport::Unsupported { println!("test skipped: Kernel has no support for SElinux context",); From 89428a77f31c26024980be35ed89e4b7de7983d7 Mon Sep 17 00:00:00 2001 From: Smicry Date: Tue, 14 Sep 2021 23:56:08 +0800 Subject: [PATCH 07/47] fix kill list final new line test --- tests/by-util/test_kill.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/tests/by-util/test_kill.rs b/tests/by-util/test_kill.rs index a16760689..91e524733 100644 --- a/tests/by-util/test_kill.rs +++ b/tests/by-util/test_kill.rs @@ -58,10 +58,8 @@ fn test_kill_list_all_signals() { #[test] fn test_kill_list_final_new_line() { - new_ucmd!() - .arg("-l") - .succeeds() - .stdout_matches(&Regex::new("[\\n\\r]$").unwrap()); + let re = Regex::new("\\n$").unwrap(); + assert!(re.is_match(new_ucmd!().arg("-l").succeeds().stdout_str())); } #[test] From 94fbe1edef0c4ac9719340b10d0ce5c5e135aa7d Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Sun, 19 Sep 2021 11:25:00 +0200 Subject: [PATCH 08/47] `chroot`: quick fix for #2687 * fixes #2687 * change error message to better mimic GNU --- src/uu/chroot/src/chroot.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs index 40799d009..c78172025 100644 --- a/src/uu/chroot/src/chroot.rs +++ b/src/uu/chroot/src/chroot.rs @@ -67,7 +67,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // TODO: refactor the args and command matching // See: https://github.com/uutils/coreutils/pull/2365#discussion_r647849967 let command: Vec<&str> = match commands.len() { - 1 => { + 0 => { let shell: &str = match user_shell { Err(_) => default_shell, Ok(ref s) => s.as_ref(), @@ -79,10 +79,22 @@ pub fn uumain(args: impl uucore::Args) -> i32 { set_context(newroot, &matches); + assert!(!command.is_empty()); let pstatus = Command::new(command[0]) .args(&command[1..]) .status() - .unwrap_or_else(|e| crash!(1, "Cannot exec: {}", e)); + .unwrap_or_else(|e| { + // TODO: Exit status: + // 125 if chroot itself fails + // 126 if command is found but cannot be invoked + // 127 if command cannot be found + crash!( + 1, + "failed to run command {}: {}", + command[0].to_string().quote(), + e + ) + }); if pstatus.success() { 0 From 8db8d8ac4ec6eea0109fa5c584991e0823025144 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Sun, 19 Sep 2021 22:21:29 +0200 Subject: [PATCH 09/47] chroot: add 'test_default_shell` --- tests/by-util/test_chroot.rs | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/by-util/test_chroot.rs b/tests/by-util/test_chroot.rs index 65d821d01..361133ccb 100644 --- a/tests/by-util/test_chroot.rs +++ b/tests/by-util/test_chroot.rs @@ -89,3 +89,24 @@ fn test_preference_of_userspec() { println!("result.stdout = {}", result.stdout_str()); println!("result.stderr = {}", result.stderr_str()); } + +#[test] +fn test_default_shell() { + let ts = TestScenario::new(util_name!()); + let at = &ts.fixtures; + + let dir = "CHROOT_DIR"; + at.mkdir(dir); + + let shell = std::env::var("SHELL").unwrap_or_else(|_| "/bin/sh".to_string()); + let expected = format!( + "chroot: failed to run command '{}': No such file or directory", + shell + ); + + if let Ok(result) = run_ucmd_as_root(&ts, &[dir]) { + result.stderr_contains(expected); + } else { + print!("TEST SKIPPED"); + } +} From 8c0b7d1314f15bb2caec618073e7a07b0b229505 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Wed, 22 Sep 2021 11:59:43 +0200 Subject: [PATCH 10/47] chroot: move logic so it can be triggered by tests * move the command building logic before the `chroot` syscall so it will be reachable by tests that don't have root permissions. --- src/uu/chroot/src/chroot.rs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/uu/chroot/src/chroot.rs b/src/uu/chroot/src/chroot.rs index c78172025..55097c1bb 100644 --- a/src/uu/chroot/src/chroot.rs +++ b/src/uu/chroot/src/chroot.rs @@ -77,11 +77,15 @@ pub fn uumain(args: impl uucore::Args) -> i32 { _ => commands, }; + assert!(!command.is_empty()); + let chroot_command = command[0]; + let chroot_args = &command[1..]; + + // NOTE: Tests can only trigger code beyond this point if they're invoked with root permissions set_context(newroot, &matches); - assert!(!command.is_empty()); - let pstatus = Command::new(command[0]) - .args(&command[1..]) + let pstatus = Command::new(chroot_command) + .args(chroot_args) .status() .unwrap_or_else(|e| { // TODO: Exit status: From 8f229aad87187b4ffe11cfbdbb747029fff6c210 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Wed, 22 Sep 2021 12:24:27 +0200 Subject: [PATCH 11/47] ls: move SELinux String building logic to its own function --- src/uu/ls/src/ls.rs | 84 ++++++++++++++++++++++++--------------------- 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 911323715..ba666d740 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1259,45 +1259,7 @@ impl PathData { None => OnceCell::new(), }; - let substitute_string = "?".to_string(); - let security_context = if config.context { - if config.selinux_supported { - #[cfg(feature = "selinux")] - { - match selinux::SecurityContext::of_path(&p_buf, must_dereference, false) { - Err(_r) => { - // TODO: show the actual reason why it failed - show_warning!("failed to get security context of: {}", p_buf.quote()); - substitute_string - } - Ok(None) => substitute_string, - Ok(Some(context)) => { - let mut context = context.as_bytes(); - if context.ends_with(&[0]) { - // TODO: replace with `strip_prefix()` when MSRV >= 1.51 - context = &context[..context.len() - 1] - }; - String::from_utf8(context.to_vec()).unwrap_or_else(|e| { - show_warning!( - "getting security context of: {}: {}", - p_buf.quote(), - e.to_string() - ); - String::from_utf8_lossy(context).into_owned() - }) - } - } - } - #[cfg(not(feature = "selinux"))] - { - substitute_string - } - } else { - substitute_string - } - } else { - String::new() - }; + let security_context = get_security_context(config, &p_buf, must_dereference); Self { md: OnceCell::new(), @@ -2124,3 +2086,47 @@ fn display_symlink_count(_metadata: &Metadata) -> String { fn display_symlink_count(metadata: &Metadata) -> String { metadata.nlink().to_string() } + +// This returns the SELinux security context as UTF8 `String`. +// In the long term this should be changed to `OsStr`, see discussions at #2621/#2656 +fn get_security_context(config: &Config, p_buf: &PathBuf, must_dereference: bool) -> String { + let substitute_string = "?".to_string(); + if config.context { + if config.selinux_supported { + #[cfg(feature = "selinux")] + { + match selinux::SecurityContext::of_path(p_buf, must_dereference, false) { + Err(_r) => { + // TODO: show the actual reason why it failed + show_warning!("failed to get security context of: {}", p_buf.quote()); + substitute_string + } + Ok(None) => substitute_string, + Ok(Some(context)) => { + let mut context = context.as_bytes(); + if context.ends_with(&[0]) { + // TODO: replace with `strip_prefix()` when MSRV >= 1.51 + context = &context[..context.len() - 1] + }; + String::from_utf8(context.to_vec()).unwrap_or_else(|e| { + show_warning!( + "getting security context of: {}: {}", + p_buf.quote(), + e.to_string() + ); + String::from_utf8_lossy(context).into_owned() + }) + } + } + } + #[cfg(not(feature = "selinux"))] + { + substitute_string + } + } else { + substitute_string + } + } else { + String::new() + } +} From 8cd8c25b0dd59aef697dbf772944fc6d57967cda Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Wed, 22 Sep 2021 13:49:08 +0200 Subject: [PATCH 12/47] ls: silence clippy warnings if feat_selinx is not set --- src/uu/ls/src/ls.rs | 69 +++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index ba666d740..51dc0d247 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1259,7 +1259,11 @@ impl PathData { None => OnceCell::new(), }; - let security_context = get_security_context(config, &p_buf, must_dereference); + let security_context = if config.context { + get_security_context(config, &p_buf, must_dereference) + } else { + String::new() + }; Self { md: OnceCell::new(), @@ -2089,44 +2093,41 @@ fn display_symlink_count(metadata: &Metadata) -> String { // This returns the SELinux security context as UTF8 `String`. // In the long term this should be changed to `OsStr`, see discussions at #2621/#2656 -fn get_security_context(config: &Config, p_buf: &PathBuf, must_dereference: bool) -> String { +#[allow(unused_variables)] +fn get_security_context(config: &Config, p_buf: &Path, must_dereference: bool) -> String { let substitute_string = "?".to_string(); - if config.context { - if config.selinux_supported { - #[cfg(feature = "selinux")] - { - match selinux::SecurityContext::of_path(p_buf, must_dereference, false) { - Err(_r) => { - // TODO: show the actual reason why it failed - show_warning!("failed to get security context of: {}", p_buf.quote()); - substitute_string - } - Ok(None) => substitute_string, - Ok(Some(context)) => { - let mut context = context.as_bytes(); - if context.ends_with(&[0]) { - // TODO: replace with `strip_prefix()` when MSRV >= 1.51 - context = &context[..context.len() - 1] - }; - String::from_utf8(context.to_vec()).unwrap_or_else(|e| { - show_warning!( - "getting security context of: {}: {}", - p_buf.quote(), - e.to_string() - ); - String::from_utf8_lossy(context).into_owned() - }) - } + if config.selinux_supported { + #[cfg(feature = "selinux")] + { + match selinux::SecurityContext::of_path(p_buf, must_dereference, false) { + Err(_r) => { + // TODO: show the actual reason why it failed + show_warning!("failed to get security context of: {}", p_buf.quote()); + substitute_string + } + Ok(None) => substitute_string, + Ok(Some(context)) => { + let mut context = context.as_bytes(); + if context.ends_with(&[0]) { + // TODO: replace with `strip_prefix()` when MSRV >= 1.51 + context = &context[..context.len() - 1] + }; + String::from_utf8(context.to_vec()).unwrap_or_else(|e| { + show_warning!( + "getting security context of: {}: {}", + p_buf.quote(), + e.to_string() + ); + String::from_utf8_lossy(context).into_owned() + }) } } - #[cfg(not(feature = "selinux"))] - { - substitute_string - } - } else { + } + #[cfg(not(feature = "selinux"))] + { substitute_string } } else { - String::new() + substitute_string } } From 30c8a4378893d52161b3f9fbea244fdcd54df52a Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Wed, 22 Sep 2021 12:03:22 +0200 Subject: [PATCH 13/47] test_chroot: add comments --- tests/by-util/test_chroot.rs | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/by-util/test_chroot.rs b/tests/by-util/test_chroot.rs index 361133ccb..3e5c22679 100644 --- a/tests/by-util/test_chroot.rs +++ b/tests/by-util/test_chroot.rs @@ -15,6 +15,7 @@ fn test_missing_operand() { #[test] fn test_enter_chroot_fails() { + // NOTE: since #2689 this test also ensures that we don't regress #2687 let (at, mut ucmd) = at_and_ucmd!(); at.mkdir("jail"); @@ -92,6 +93,7 @@ fn test_preference_of_userspec() { #[test] fn test_default_shell() { + // NOTE: This test intends to trigger code which can only be reached with root permissions. let ts = TestScenario::new(util_name!()); let at = &ts.fixtures; @@ -99,14 +101,15 @@ fn test_default_shell() { at.mkdir(dir); let shell = std::env::var("SHELL").unwrap_or_else(|_| "/bin/sh".to_string()); - let expected = format!( + let _expected = format!( "chroot: failed to run command '{}': No such file or directory", shell ); - if let Ok(result) = run_ucmd_as_root(&ts, &[dir]) { - result.stderr_contains(expected); - } else { - print!("TEST SKIPPED"); - } + // TODO: [2021-09; jhscheer] uncomment if/when #2692 gets merged + // if let Ok(result) = run_ucmd_as_root(&ts, &[dir]) { + // result.stderr_contains(expected); + // } else { + // print!("TEST SKIPPED"); + // } } From 3882df5cdcde6e96f559176f9936f49e81a966b2 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 25 Sep 2021 00:22:01 -0300 Subject: [PATCH 14/47] expr: use UResult --- src/uu/expr/src/expr.rs | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/src/uu/expr/src/expr.rs b/src/uu/expr/src/expr.rs index 2d82300ff..eaf329bc5 100644 --- a/src/uu/expr/src/expr.rs +++ b/src/uu/expr/src/expr.rs @@ -9,6 +9,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; +use uucore::error::{UResult, USimpleError}; use uucore::InvalidEncodingHandling; mod syntax_tree; @@ -23,7 +24,8 @@ pub fn uu_app() -> App<'static, 'static> { .arg(Arg::with_name(HELP).long(HELP)) } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let args = args .collect_str(InvalidEncodingHandling::ConvertLossy) .accept_any(); @@ -32,13 +34,13 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // The following usage should work without escaping hyphens: `expr -15 = 1 + 2 \* \( 3 - -4 \)` if maybe_handle_help_or_version(&args) { - 0 + Ok(()) } else { let token_strings = args[1..].to_vec(); match process_expr(&token_strings) { Ok(expr_result) => print_expr_ok(&expr_result), - Err(expr_error) => print_expr_error(&expr_error), + Err(expr_error) => Err(USimpleError::new(2, &expr_error)), } } } @@ -49,19 +51,15 @@ fn process_expr(token_strings: &[String]) -> Result { evaluate_ast(maybe_ast) } -fn print_expr_ok(expr_result: &str) -> i32 { +fn print_expr_ok(expr_result: &str) -> UResult<()> { println!("{}", expr_result); if expr_result == "0" || expr_result.is_empty() { - 1 + Err(1.into()) } else { - 0 + Ok(()) } } -fn print_expr_error(expr_error: &str) -> ! { - crash!(2, "{}", expr_error) -} - fn evaluate_ast(maybe_ast: Result, String>) -> Result { maybe_ast.and_then(|ast| ast.evaluate()) } From 4ff5fea502a87c7334d5d954d47c1a1def4e48af Mon Sep 17 00:00:00 2001 From: vulppine Date: Fri, 1 Oct 2021 14:58:26 -0700 Subject: [PATCH 15/47] cp: uumain returns UResult, UError for Error --- src/uu/cp/src/cp.rs | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index cd33f9fa6..97a196dc1 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -49,6 +49,7 @@ use std::path::{Path, PathBuf, StripPrefixError}; use std::str::FromStr; use std::string::ToString; use uucore::backup_control::{self, BackupMode}; +use uucore::error::{set_exit_code, UError, UResult}; use uucore::fs::{canonicalize, MissingHandling, ResolveMode}; use walkdir::WalkDir; @@ -105,6 +106,12 @@ quick_error! { } } +impl UError for Error { + fn code(&self) -> i32 { + 1 + } +} + /// Continue next iteration of loop if result of expression is error macro_rules! or_continue( ($expr:expr) => (match $expr { @@ -220,7 +227,6 @@ pub struct Options { static ABOUT: &str = "Copy SOURCE to DEST, or multiple SOURCE(s) to DIRECTORY."; static LONG_HELP: &str = ""; -static EXIT_OK: i32 = 0; static EXIT_ERR: i32 = 1; fn usage() -> String { @@ -446,7 +452,7 @@ pub fn uu_app() -> App<'static, 'static> { .multiple(true)) } -pub fn uumain(args: impl uucore::Args) -> i32 { +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let usage = usage(); let matches = uu_app() .after_help(&*format!( @@ -457,11 +463,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .usage(&usage[..]) .get_matches_from(args); - let options = crash_if_err!(EXIT_ERR, Options::from_matches(&matches)); + let options = Options::from_matches(&matches)?; if options.overwrite == OverwriteMode::NoClobber && options.backup != BackupMode::NoBackup { show_usage_error!("options --backup and --no-clobber are mutually exclusive"); - return 1; + set_exit_code(EXIT_ERR); + return Ok(()); } let paths: Vec = matches @@ -469,7 +476,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .map(|v| v.map(ToString::to_string).collect()) .unwrap_or_default(); - let (sources, target) = crash_if_err!(EXIT_ERR, parse_path_args(&paths, &options)); + let (sources, target) = parse_path_args(&paths, &options)?; if let Err(error) = copy(&sources, &target, &options) { match error { @@ -479,10 +486,10 @@ pub fn uumain(args: impl uucore::Args) -> i32 { // Else we caught a fatal bubbled-up error, log it to stderr _ => show_error!("{}", error), }; - return EXIT_ERR; + set_exit_code(EXIT_ERR); } - EXIT_OK + Ok(()) } impl ClobberMode { @@ -1124,7 +1131,7 @@ fn copy_attribute(source: &Path, dest: &Path, attribute: &Attribute) -> CopyResu let xattrs = xattr::list(source)?; for attr in xattrs { if let Some(attr_value) = xattr::get(source, attr.clone())? { - crash_if_err!(EXIT_ERR, xattr::set(dest, attr, &attr_value[..])); + xattr::set(dest, attr, &attr_value[..])?; } } } From c64f09dc59897583d5cf7fac856b2daa69cc6089 Mon Sep 17 00:00:00 2001 From: vulppine Date: Fri, 1 Oct 2021 15:18:05 -0700 Subject: [PATCH 16/47] cp: Adds a needed macro, changes a return --- src/uu/cp/src/cp.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index 97a196dc1..b50bca905 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -49,7 +49,7 @@ use std::path::{Path, PathBuf, StripPrefixError}; use std::str::FromStr; use std::string::ToString; use uucore::backup_control::{self, BackupMode}; -use uucore::error::{set_exit_code, UError, UResult}; +use uucore::error::{set_exit_code, ExitCode, UError, UResult}; use uucore::fs::{canonicalize, MissingHandling, ResolveMode}; use walkdir::WalkDir; @@ -452,6 +452,7 @@ pub fn uu_app() -> App<'static, 'static> { .multiple(true)) } +#[uucore_procs::gen_uumain] pub fn uumain(args: impl uucore::Args) -> UResult<()> { let usage = usage(); let matches = uu_app() @@ -467,8 +468,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { if options.overwrite == OverwriteMode::NoClobber && options.backup != BackupMode::NoBackup { show_usage_error!("options --backup and --no-clobber are mutually exclusive"); - set_exit_code(EXIT_ERR); - return Ok(()); + return Err(ExitCode(EXIT_ERR).into()); } let paths: Vec = matches From e9371dc57d6f323f9d8953126af9fa23d81054b5 Mon Sep 17 00:00:00 2001 From: Jan Scheer Date: Fri, 1 Oct 2021 23:25:56 +0200 Subject: [PATCH 17/47] common/util: fix parsing of coreutil version For the CICD on macOS, this fixes: ``` ---- common::util::tests::test_check_coreutil_version stdout ---- ---- common::util::tests::test_expected_result stdout ---- thread 'common::util::tests::test_expected_result' panicked at 'byte index 4 is out of bounds of `9.0`', tests/common/util.rs:1172:41 ``` --- tests/common/util.rs | 46 +++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/tests/common/util.rs b/tests/common/util.rs index f3cdec010..8e9078e9c 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -1169,7 +1169,7 @@ pub fn check_coreutil_version( if s.contains(&format!("(GNU coreutils) {}", version_expected)) { Ok(format!("{}: {}", UUTILS_INFO, s.to_string())) } else if s.contains("(GNU coreutils)") { - let version_found = s.split_whitespace().last().unwrap()[..4].parse::().unwrap_or_default(); + let version_found = parse_coreutil_version(s); let version_expected = version_expected.parse::().unwrap_or_default(); if version_found > version_expected { Ok(format!("{}: version for the reference coreutil '{}' is higher than expected; expected: {}, found: {}", UUTILS_INFO, util_name, version_expected, version_found)) @@ -1182,6 +1182,20 @@ pub fn check_coreutil_version( ) } +// simple heuristic to parse the coreutils SemVer string, e.g. "id (GNU coreutils) 8.32.263-0475" +fn parse_coreutil_version(version_string: &str) -> f32 { + version_string + .split_whitespace() + .last() + .unwrap() + .split('.') + .take(2) + .collect::>() + .join(".") + .parse::() + .unwrap_or_default() +} + /// This runs the GNU coreutils `util_name` binary in `$PATH` in order to /// dynamically gather reference values on the system. /// If the `util_name` in `$PATH` doesn't include a coreutils version string, @@ -1474,6 +1488,36 @@ mod tests { res.normalized_newlines_stdout_is("A\r\nB\nC\n"); } + #[test] + #[cfg(unix)] + fn test_parse_coreutil_version() { + use std::assert_eq; + assert_eq!( + parse_coreutil_version("id (GNU coreutils) 9.0.123-0123").to_string(), + "9" + ); + assert_eq!( + parse_coreutil_version("id (GNU coreutils) 8.32.263-0475").to_string(), + "8.32" + ); + assert_eq!( + parse_coreutil_version("id (GNU coreutils) 8.25.123-0123").to_string(), + "8.25" + ); + assert_eq!( + parse_coreutil_version("id (GNU coreutils) 9.0").to_string(), + "9" + ); + assert_eq!( + parse_coreutil_version("id (GNU coreutils) 8.32").to_string(), + "8.32" + ); + assert_eq!( + parse_coreutil_version("id (GNU coreutils) 8.25").to_string(), + "8.25" + ); + } + #[test] #[cfg(unix)] fn test_check_coreutil_version() { From 4319248bb650bda453a17e8f2d30d6b922ed700d Mon Sep 17 00:00:00 2001 From: vulppine Date: Fri, 1 Oct 2021 16:45:19 -0700 Subject: [PATCH 18/47] cp: Changes '1' to 'EXIT_ERR' in UError impl --- src/uu/cp/src/cp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/cp/src/cp.rs b/src/uu/cp/src/cp.rs index b50bca905..518a2262c 100644 --- a/src/uu/cp/src/cp.rs +++ b/src/uu/cp/src/cp.rs @@ -108,7 +108,7 @@ quick_error! { impl UError for Error { fn code(&self) -> i32 { - 1 + EXIT_ERR } } From d013461a6fa963c130468d4c7e26ddd437082b9c Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 1 Oct 2021 22:27:01 -0400 Subject: [PATCH 19/47] ls: replace redundant closure with function itself --- src/uu/ls/src/ls.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 51dc0d247..3fa3b4f8e 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -1967,11 +1967,7 @@ fn display_file_name( #[cfg(unix)] { if config.format != Format::Long && config.inode { - name = path - .md() - .map_or_else(|| "?".to_string(), |md| get_inode(md)) - + " " - + &name; + name = path.md().map_or_else(|| "?".to_string(), get_inode) + " " + &name; } } From 71b7d6b57da8853059d8a78e115ac3dbce27280d Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 1 Oct 2021 22:27:19 -0400 Subject: [PATCH 20/47] more: remove redundant mut from stdout accesses --- src/uu/more/src/more.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/uu/more/src/more.rs b/src/uu/more/src/more.rs index 3a601c1e8..d424d5a77 100644 --- a/src/uu/more/src/more.rs +++ b/src/uu/more/src/more.rs @@ -210,7 +210,7 @@ fn reset_term(stdout: &mut std::io::Stdout) { #[inline(always)] fn reset_term(_: &mut usize) {} -fn more(buff: &str, mut stdout: &mut Stdout, next_file: Option<&str>, silent: bool) { +fn more(buff: &str, stdout: &mut Stdout, next_file: Option<&str>, silent: bool) { let (cols, rows) = terminal::size().unwrap(); let lines = break_buff(buff, usize::from(cols)); @@ -232,7 +232,7 @@ fn more(buff: &str, mut stdout: &mut Stdout, next_file: Option<&str>, silent: bo code: KeyCode::Char('c'), modifiers: KeyModifiers::CONTROL, }) => { - reset_term(&mut stdout); + reset_term(stdout); std::process::exit(0); } Event::Key(KeyEvent { From 06ae968ecf732ce53954f01b85fbae5969a49d38 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 1 Oct 2021 23:01:06 -0400 Subject: [PATCH 21/47] csplit: use assert! instead of if then panic! --- src/uu/csplit/src/csplit.rs | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/src/uu/csplit/src/csplit.rs b/src/uu/csplit/src/csplit.rs index dbf65b71d..0d99154df 100644 --- a/src/uu/csplit/src/csplit.rs +++ b/src/uu/csplit/src/csplit.rs @@ -320,18 +320,19 @@ impl<'a> SplitWriter<'a> { let l = line?; match n.cmp(&(&ln + 1)) { Ordering::Less => { - if input_iter.add_line_to_buffer(ln, l).is_some() { - panic!("the buffer is big enough to contain 1 line"); - } + assert!( + input_iter.add_line_to_buffer(ln, l).is_none(), + "the buffer is big enough to contain 1 line" + ); ret = Ok(()); break; } Ordering::Equal => { - if !self.options.suppress_matched - && input_iter.add_line_to_buffer(ln, l).is_some() - { - panic!("the buffer is big enough to contain 1 line"); - } + assert!( + self.options.suppress_matched + || input_iter.add_line_to_buffer(ln, l).is_none(), + "the buffer is big enough to contain 1 line" + ); ret = Ok(()); break; } @@ -378,9 +379,10 @@ impl<'a> SplitWriter<'a> { match (self.options.suppress_matched, offset) { // no offset, add the line to the next split (false, 0) => { - if input_iter.add_line_to_buffer(ln, l).is_some() { - panic!("the buffer is big enough to contain 1 line"); - } + assert!( + input_iter.add_line_to_buffer(ln, l).is_none(), + "the buffer is big enough to contain 1 line" + ); } // a positive offset, some more lines need to be added to the current split (false, _) => self.writeln(l)?, @@ -425,9 +427,10 @@ impl<'a> SplitWriter<'a> { if !self.options.suppress_matched { // add 1 to the buffer size to make place for the matched line input_iter.set_size_of_buffer(offset_usize + 1); - if input_iter.add_line_to_buffer(ln, l).is_some() { - panic!("should be big enough to hold every lines"); - } + assert!( + input_iter.add_line_to_buffer(ln, l).is_none(), + "should be big enough to hold every lines" + ); } self.finish_split(); if input_iter.buffer_len() < offset_usize { From de158c012243554b71ced15f35eca45fd1a86c80 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 1 Oct 2021 23:02:17 -0400 Subject: [PATCH 22/47] sort: replace redundant closure with function itself --- src/uu/sort/src/sort.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/sort/src/sort.rs b/src/uu/sort/src/sort.rs index fe286aa6d..bd79a6811 100644 --- a/src/uu/sort/src/sort.rs +++ b/src/uu/sort/src/sort.rs @@ -825,7 +825,7 @@ impl FieldSelector { fn parse(key: &str, global_settings: &GlobalSettings) -> UResult { let mut from_to = key.split(','); let (from, from_options) = Self::split_key_options(from_to.next().unwrap()); - let to = from_to.next().map(|to| Self::split_key_options(to)); + let to = from_to.next().map(Self::split_key_options); let options_are_empty = from_options.is_empty() && matches!(to, None | Some((_, ""))); if options_are_empty { From 6aee05a0f141b935f646ec1cd874feab93c5ebf0 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 2 Oct 2021 09:44:16 -0400 Subject: [PATCH 23/47] od: use assert! instead of if then panic! --- src/uu/od/src/output_info.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/uu/od/src/output_info.rs b/src/uu/od/src/output_info.rs index 49c2a09a2..cf050475a 100644 --- a/src/uu/od/src/output_info.rs +++ b/src/uu/od/src/output_info.rs @@ -145,13 +145,12 @@ impl OutputInfo { byte_size_block: usize, print_width_block: usize, ) -> [usize; MAX_BYTES_PER_UNIT] { - if byte_size_block > MAX_BYTES_PER_UNIT { - panic!( - "{}-bits types are unsupported. Current max={}-bits.", - 8 * byte_size_block, - 8 * MAX_BYTES_PER_UNIT - ); - } + assert!( + byte_size_block <= MAX_BYTES_PER_UNIT, + "{}-bits types are unsupported. Current max={}-bits.", + 8 * byte_size_block, + 8 * MAX_BYTES_PER_UNIT + ); let mut spacing = [0; MAX_BYTES_PER_UNIT]; let mut byte_size = sf.byte_size(); From 548a5121ae5bbab718896688dc8cebfe30f1bdc0 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Sat, 2 Oct 2021 10:15:15 -0400 Subject: [PATCH 24/47] dd: use assert! instead of if then panic! --- src/uu/dd/src/parseargs/unit_tests.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/uu/dd/src/parseargs/unit_tests.rs b/src/uu/dd/src/parseargs/unit_tests.rs index b898f1e5d..21900ee49 100644 --- a/src/uu/dd/src/parseargs/unit_tests.rs +++ b/src/uu/dd/src/parseargs/unit_tests.rs @@ -35,12 +35,11 @@ fn unimplemented_flags_should_error_non_linux() { } } - if !succeeded.is_empty() { - panic!( - "The following flags did not panic as expected: {:?}", - succeeded - ); - } + assert!( + succeeded.is_empty(), + "The following flags did not panic as expected: {:?}", + succeeded + ); } #[test] @@ -64,12 +63,11 @@ fn unimplemented_flags_should_error() { } } - if !succeeded.is_empty() { - panic!( - "The following flags did not panic as expected: {:?}", - succeeded - ); - } + assert!( + succeeded.is_empty(), + "The following flags did not panic as expected: {:?}", + succeeded + ); } #[test] From d5caa0d9d88c7d19cee46b0a4137397ffa7c5642 Mon Sep 17 00:00:00 2001 From: vulppine Date: Sat, 2 Oct 2021 08:15:25 -0700 Subject: [PATCH 25/47] seq: Adds hexadecimal integer parsing --- src/uu/seq/src/seq.rs | 76 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 61 insertions(+), 15 deletions(-) diff --git a/src/uu/seq/src/seq.rs b/src/uu/seq/src/seq.rs index aac8f2280..594796641 100644 --- a/src/uu/seq/src/seq.rs +++ b/src/uu/seq/src/seq.rs @@ -88,30 +88,76 @@ impl FromStr for Number { if s.starts_with('+') { s = &s[1..]; } - - match s.parse::() { - Ok(n) => { - // If `s` is '-0', then `parse()` returns - // `BigInt::zero()`, but we need to return - // `Number::MinusZero` instead. - if n == BigInt::zero() && s.starts_with('-') { - Ok(Number::MinusZero) - } else { - Ok(Number::BigInt(n)) - } + let is_neg = s.starts_with('-'); + let is_hex = { + // GNU 20.11.2 - Parsing of Floats + match s.find("0x") { + Some(i) => (true, i), + None => match s.find("0X") { + Some(i) => (true, i), + None => (false, 0), + }, } - Err(_) => match s.parse::() { - Ok(value) if value.is_nan() => Err(format!( + }; + + match is_hex { + (true, i) => match i <= 1 { + false => Err(format!( + "invalid hexadecimal argument: {}\nTry '{} --help' for more information.", + s.quote(), + uucore::execution_phrase(), + )), + true => match &s.as_bytes()[i + 2] { + b'-' | b'+' => Err(format!( + "invalid hexadecimal argument: {}\nTry '{} --help' for more information.", + s.quote(), + uucore::execution_phrase(), + )), + // TODO: hexadecimal floating point parsing (see #2660) + b'.' => Err(format!( + "NotImplemented: hexadecimal floating point numbers: {}\nTry '{} --help' for more information.", + s.quote(), + uucore::execution_phrase(), + )), + _ => { + let num = BigInt::from_str_radix(&s[i + 2..], 16) + .map_err(|_| format!( + "invalid hexadecimal argument: {}\nTry '{} --help' for more information.", + s.quote(), + uucore::execution_phrase(), + ))?; + match (is_neg, num == BigInt::zero()) { + (true, true) => Ok(Number::MinusZero), + (true, false) => Ok(Number::BigInt(-num)), + (false, _) => Ok(Number::BigInt(num)), + } + } + }, + }, + (false, _) => match s.parse::() { + Ok(n) => { + // If `s` is '-0', then `parse()` returns + // `BigInt::zero()`, but we need to return + // `Number::MinusZero` instead. + if n == BigInt::zero() && is_neg { + Ok(Number::MinusZero) + } else { + Ok(Number::BigInt(n)) + } + } + Err(_) => match s.parse::() { + Ok(value) if value.is_nan() => Err(format!( "invalid 'not-a-number' argument: {}\nTry '{} --help' for more information.", s.quote(), uucore::execution_phrase(), )), - Ok(value) => Ok(Number::F64(value)), - Err(_) => Err(format!( + Ok(value) => Ok(Number::F64(value)), + Err(_) => Err(format!( "invalid floating point argument: {}\nTry '{} --help' for more information.", s.quote(), uucore::execution_phrase(), )), + }, }, } } From aad0682a404814a03d47efb8bcb88c7b14f8bcdb Mon Sep 17 00:00:00 2001 From: vulppine Date: Sat, 2 Oct 2021 08:46:09 -0700 Subject: [PATCH 26/47] seq: Adds testing for hexadecimal integer parsing --- tests/by-util/test_seq.rs | 40 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/tests/by-util/test_seq.rs b/tests/by-util/test_seq.rs index 27b5f99bc..2d2ea5344 100644 --- a/tests/by-util/test_seq.rs +++ b/tests/by-util/test_seq.rs @@ -1,6 +1,46 @@ use crate::common::util::*; use std::io::Read; +#[test] +fn test_hex_rejects_posneg_after_identifier() { + new_ucmd!() + .args(&["0x-123ABC"]) + .fails() + .no_stdout() + .stderr_contains("invalid hexadecimal argument: '0x-123ABC'") + .stderr_contains("for more information."); + new_ucmd!() + .args(&["0x+123ABC"]) + .fails() + .no_stdout() + .stderr_contains("invalid hexadecimal argument: '0x+123ABC'") + .stderr_contains("for more information."); + new_ucmd!() + .args(&["-0x-123ABC"]) + .fails() + .no_stdout() + .stderr_contains("invalid hexadecimal argument: '-0x-123ABC'") + .stderr_contains("for more information."); + new_ucmd!() + .args(&["-0x+123ABC"]) + .fails() + .no_stdout() + .stderr_contains("invalid hexadecimal argument: '-0x+123ABC'") + .stderr_contains("for more information."); +} + +#[test] +fn test_hex_lowercase_uppercase() { + new_ucmd!() + .args(&["0xa", "0xA"]) + .succeeds() + .stdout_is("10\n"); + new_ucmd!() + .args(&["0Xa", "0XA"]) + .succeeds() + .stdout_is("10\n"); +} + #[test] fn test_rejects_nan() { let ts = TestScenario::new(util_name!()); From 9dd401c3584633376440ea61af40ec786a727077 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:10:00 -0300 Subject: [PATCH 27/47] base32: base_common use UResult --- src/uu/base32/src/base_common.rs | 78 ++++++++++++++++---------------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/src/uu/base32/src/base_common.rs b/src/uu/base32/src/base_common.rs index 0fd0fa5c4..015925e12 100644 --- a/src/uu/base32/src/base_common.rs +++ b/src/uu/base32/src/base_common.rs @@ -11,13 +11,16 @@ use std::io::{stdout, Read, Write}; use uucore::display::Quotable; use uucore::encoding::{wrap_print, Data, Format}; +use uucore::error::{FromIo, UResult, USimpleError, UUsageError}; use uucore::InvalidEncodingHandling; use std::fs::File; use std::io::{BufReader, Stdin}; use std::path::Path; -use clap::{App, Arg}; +use clap::{crate_version, App, Arg}; + +pub static BASE_CMD_PARSE_ERROR: i32 = 1; // Config. pub struct Config { @@ -35,15 +38,14 @@ pub mod options { } impl Config { - pub fn from(app_name: &str, options: &clap::ArgMatches) -> Result { + pub fn from(options: &clap::ArgMatches) -> UResult { let file: Option = match options.values_of(options::FILE) { Some(mut values) => { let name = values.next().unwrap(); if let Some(extra_op) = values.next() { - return Err(format!( - "extra operand {}\nTry '{} --help' for more information.", - extra_op.quote(), - app_name + return Err(UUsageError::new( + BASE_CMD_PARSE_ERROR, + format!("extra operand {}", extra_op.quote(),), )); } @@ -51,7 +53,10 @@ impl Config { None } else { if !Path::exists(Path::new(name)) { - return Err(format!("{}: No such file or directory", name.maybe_quote())); + return Err(USimpleError::new( + BASE_CMD_PARSE_ERROR, + format!("{}: No such file or directory", name.maybe_quote()), + )); } Some(name.to_owned()) } @@ -62,8 +67,12 @@ impl Config { let cols = options .value_of(options::WRAP) .map(|num| { - num.parse::() - .map_err(|_| format!("invalid wrap size: {}", num.quote())) + num.parse::().map_err(|_| { + USimpleError::new( + BASE_CMD_PARSE_ERROR, + format!("invalid wrap size: {}", num.quote()), + ) + }) }) .transpose()?; @@ -76,23 +85,17 @@ impl Config { } } -pub fn parse_base_cmd_args( - args: impl uucore::Args, - name: &str, - version: &str, - about: &str, - usage: &str, -) -> Result { - let app = base_app(name, version, about).usage(usage); +pub fn parse_base_cmd_args(args: impl uucore::Args, about: &str, usage: &str) -> UResult { + let app = base_app(about).usage(usage); let arg_list = args .collect_str(InvalidEncodingHandling::ConvertLossy) .accept_any(); - Config::from(name, &app.get_matches_from(arg_list)) + Config::from(&app.get_matches_from(arg_list)) } -pub fn base_app<'a>(name: &str, version: &'a str, about: &'a str) -> App<'static, 'a> { - App::new(name) - .version(version) +pub fn base_app<'a>(about: &'a str) -> App<'static, 'a> { + App::new(uucore::util_name()) + .version(crate_version!()) .about(about) // Format arguments. .arg( @@ -121,14 +124,15 @@ pub fn base_app<'a>(name: &str, version: &'a str, about: &'a str) -> App<'static .arg(Arg::with_name(options::FILE).index(1).multiple(true)) } -pub fn get_input<'a>(config: &Config, stdin_ref: &'a Stdin) -> Box { +pub fn get_input<'a>(config: &Config, stdin_ref: &'a Stdin) -> UResult> { match &config.to_read { Some(name) => { - let file_buf = crash_if_err!(1, File::open(Path::new(name))); - Box::new(BufReader::new(file_buf)) // as Box + let file_buf = + File::open(Path::new(name)).map_err_context(|| name.maybe_quote().to_string())?; + Ok(Box::new(BufReader::new(file_buf))) // as Box } None => { - Box::new(stdin_ref.lock()) // as Box + Ok(Box::new(stdin_ref.lock())) // as Box } } } @@ -139,8 +143,7 @@ pub fn handle_input( line_wrap: Option, ignore_garbage: bool, decode: bool, - name: &str, -) { +) -> UResult<()> { let mut data = Data::new(input, format).ignore_garbage(ignore_garbage); if let Some(wrap) = line_wrap { data = data.line_wrap(wrap); @@ -150,28 +153,23 @@ pub fn handle_input( match data.encode() { Ok(s) => { wrap_print(&data, s); + Ok(()) } - Err(_) => { - eprintln!( - "{}: error: invalid input (length must be multiple of 4 characters)", - name - ); - exit!(1) - } + Err(_) => Err(USimpleError::new( + 1, + "error: invalid input (length must be multiple of 4 characters)", + )), } } else { match data.decode() { Ok(s) => { if stdout().write_all(&s).is_err() { // on windows console, writing invalid utf8 returns an error - eprintln!("{}: error: Cannot write non-utf8 data", name); - exit!(1) + return Err(USimpleError::new(1, "error: cannot write non-utf8 data")); } + Ok(()) } - Err(_) => { - eprintln!("{}: error: invalid input", name); - exit!(1) - } + Err(_) => Err(USimpleError::new(1, "error: invalid input")), } } } From 97df700d67acb6d6c94859a713445aaffbdbbb67 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:12:09 -0300 Subject: [PATCH 28/47] base32: use UResult --- src/uu/base32/src/base32.rs | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/uu/base32/src/base32.rs b/src/uu/base32/src/base32.rs index 667fd927e..0159ef181 100644 --- a/src/uu/base32/src/base32.rs +++ b/src/uu/base32/src/base32.rs @@ -11,7 +11,7 @@ extern crate uucore; use std::io::{stdin, Read}; use clap::App; -use uucore::encoding::Format; +use uucore::{encoding::Format, error::UResult}; pub mod base_common; @@ -24,27 +24,22 @@ static ABOUT: &str = " to attempt to recover from any other non-alphabet bytes in the encoded stream. "; -static VERSION: &str = env!("CARGO_PKG_VERSION"); - -static BASE_CMD_PARSE_ERROR: i32 = 1; fn usage() -> String { format!("{0} [OPTION]... [FILE]", uucore::execution_phrase()) } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let format = Format::Base32; let usage = usage(); - let name = uucore::util_name(); - let config_result: Result = - base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); - let config = config_result.unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)); + let config: base_common::Config = base_common::parse_base_cmd_args(args, ABOUT, &usage)?; // Create a reference to stdin so we can return a locked stdin from // parse_base_cmd_args let stdin_raw = stdin(); - let mut input: Box = base_common::get_input(&config, &stdin_raw); + let mut input: Box = base_common::get_input(&config, &stdin_raw)?; base_common::handle_input( &mut input, @@ -52,12 +47,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.wrap_cols, config.ignore_garbage, config.decode, - name, - ); - - 0 + ) } pub fn uu_app() -> App<'static, 'static> { - base_common::base_app(uucore::util_name(), VERSION, ABOUT) + base_common::base_app(ABOUT) } From 051284de4ce1322ce0574b4940f37199cfdf0b14 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:12:42 -0300 Subject: [PATCH 29/47] tests/base32: update test --- tests/by-util/test_base32.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_base32.rs b/tests/by-util/test_base32.rs index 5c74c5b59..ffe2cf74c 100644 --- a/tests/by-util/test_base32.rs +++ b/tests/by-util/test_base32.rs @@ -113,12 +113,18 @@ fn test_wrap_bad_arg() { #[test] fn test_base32_extra_operand() { + let ts = TestScenario::new(util_name!()); + // Expect a failure when multiple files are specified. - new_ucmd!() + ts.ucmd() .arg("a.txt") .arg("b.txt") .fails() - .stderr_only("base32: extra operand 'b.txt'\nTry 'base32 --help' for more information."); + .stderr_only(format!( + "{0}: extra operand 'b.txt'\nTry '{1} {0} --help' for more information.", + ts.util_name, + ts.bin_path.to_string_lossy() + )); } #[test] From 452329ad199878e9f483c67621791f94cbc2eaa2 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:15:40 -0300 Subject: [PATCH 30/47] base64: use UResult --- src/uu/base64/src/base64.rs | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/uu/base64/src/base64.rs b/src/uu/base64/src/base64.rs index ded157362..ed8b1b461 100644 --- a/src/uu/base64/src/base64.rs +++ b/src/uu/base64/src/base64.rs @@ -12,7 +12,7 @@ extern crate uucore; use uu_base32::base_common; pub use uu_base32::uu_app; -use uucore::encoding::Format; +use uucore::{encoding::Format, error::UResult}; use std::io::{stdin, Read}; @@ -25,26 +25,22 @@ static ABOUT: &str = " to attempt to recover from any other non-alphabet bytes in the encoded stream. "; -static VERSION: &str = env!("CARGO_PKG_VERSION"); - -static BASE_CMD_PARSE_ERROR: i32 = 1; fn usage() -> String { format!("{0} [OPTION]... [FILE]", uucore::execution_phrase()) } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let format = Format::Base64; let usage = usage(); - let name = uucore::util_name(); - let config_result: Result = - base_common::parse_base_cmd_args(args, name, VERSION, ABOUT, &usage); - let config = config_result.unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)); + + let config: base_common::Config = base_common::parse_base_cmd_args(args, ABOUT, &usage)?; // Create a reference to stdin so we can return a locked stdin from // parse_base_cmd_args let stdin_raw = stdin(); - let mut input: Box = base_common::get_input(&config, &stdin_raw); + let mut input: Box = base_common::get_input(&config, &stdin_raw)?; base_common::handle_input( &mut input, @@ -52,8 +48,5 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.wrap_cols, config.ignore_garbage, config.decode, - name, - ); - - 0 + ) } From f85ccf8e460dca0a11f4a7a8440aa62e02964952 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:16:07 -0300 Subject: [PATCH 31/47] tests/base64: update test --- tests/by-util/test_base64.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/by-util/test_base64.rs b/tests/by-util/test_base64.rs index a860aae91..87aa0db44 100644 --- a/tests/by-util/test_base64.rs +++ b/tests/by-util/test_base64.rs @@ -95,12 +95,18 @@ fn test_wrap_bad_arg() { #[test] fn test_base64_extra_operand() { + let ts = TestScenario::new(util_name!()); + // Expect a failure when multiple files are specified. - new_ucmd!() + ts.ucmd() .arg("a.txt") .arg("b.txt") .fails() - .stderr_only("base64: extra operand 'b.txt'\nTry 'base64 --help' for more information."); + .stderr_only(format!( + "{0}: extra operand 'b.txt'\nTry '{1} {0} --help' for more information.", + ts.util_name, + ts.bin_path.to_string_lossy() + )); } #[test] From b924774c8a2e6cedffa22ab5e8f429d885362c46 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sat, 2 Oct 2021 23:17:16 -0300 Subject: [PATCH 32/47] basenc: use UResult --- src/uu/basenc/src/basenc.rs | 40 ++++++++++++++++--------------------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/src/uu/basenc/src/basenc.rs b/src/uu/basenc/src/basenc.rs index 86c251ad1..a730d3e0f 100644 --- a/src/uu/basenc/src/basenc.rs +++ b/src/uu/basenc/src/basenc.rs @@ -11,10 +11,14 @@ #[macro_use] extern crate uucore; -use clap::{crate_version, App, Arg}; -use uu_base32::base_common::{self, Config}; +use clap::{App, Arg}; +use uu_base32::base_common::{self, Config, BASE_CMD_PARSE_ERROR}; -use uucore::{encoding::Format, InvalidEncodingHandling}; +use uucore::{ + encoding::Format, + error::{UResult, UUsageError}, + InvalidEncodingHandling, +}; use std::io::{stdin, Read}; @@ -26,8 +30,6 @@ static ABOUT: &str = " from any other non-alphabet bytes in the encoded stream. "; -static BASE_CMD_PARSE_ERROR: i32 = 1; - const ENCODINGS: &[(&str, Format)] = &[ ("base64", Format::Base64), ("base64url", Format::Base64Url), @@ -47,14 +49,14 @@ fn usage() -> String { } pub fn uu_app() -> App<'static, 'static> { - let mut app = base_common::base_app(uucore::util_name(), crate_version!(), ABOUT); + let mut app = base_common::base_app(ABOUT); for encoding in ENCODINGS { app = app.arg(Arg::with_name(encoding.0).long(encoding.0)); } app } -fn parse_cmd_args(args: impl uucore::Args) -> (Config, Format) { +fn parse_cmd_args(args: impl uucore::Args) -> UResult<(Config, Format)> { let usage = usage(); let matches = uu_app().usage(&usage[..]).get_matches_from( args.collect_str(InvalidEncodingHandling::ConvertLossy) @@ -63,24 +65,19 @@ fn parse_cmd_args(args: impl uucore::Args) -> (Config, Format) { let format = ENCODINGS .iter() .find(|encoding| matches.is_present(encoding.0)) - .unwrap_or_else(|| { - show_usage_error!("missing encoding type"); - std::process::exit(1) - }) + .ok_or_else(|| UUsageError::new(BASE_CMD_PARSE_ERROR, "missing encoding type"))? .1; - ( - Config::from("basenc", &matches).unwrap_or_else(|s| crash!(BASE_CMD_PARSE_ERROR, "{}", s)), - format, - ) + let config = Config::from(&matches)?; + Ok((config, format)) } -pub fn uumain(args: impl uucore::Args) -> i32 { - let name = uucore::util_name(); - let (config, format) = parse_cmd_args(args); +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { + let (config, format) = parse_cmd_args(args)?; // Create a reference to stdin so we can return a locked stdin from // parse_base_cmd_args let stdin_raw = stdin(); - let mut input: Box = base_common::get_input(&config, &stdin_raw); + let mut input: Box = base_common::get_input(&config, &stdin_raw)?; base_common::handle_input( &mut input, @@ -88,8 +85,5 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.wrap_cols, config.ignore_garbage, config.decode, - name, - ); - - 0 + ) } From 4e1f945e86e99e350f3f3e12db8057d19401acc2 Mon Sep 17 00:00:00 2001 From: vulppine Date: Sun, 3 Oct 2021 09:50:49 -0700 Subject: [PATCH 33/47] seq: Adds testing for large hex numbers --- tests/by-util/test_seq.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/by-util/test_seq.rs b/tests/by-util/test_seq.rs index 2d2ea5344..7136f5e76 100644 --- a/tests/by-util/test_seq.rs +++ b/tests/by-util/test_seq.rs @@ -2,7 +2,7 @@ use crate::common::util::*; use std::io::Read; #[test] -fn test_hex_rejects_posneg_after_identifier() { +fn test_hex_rejects_sign_after_identifier() { new_ucmd!() .args(&["0x-123ABC"]) .fails() @@ -41,6 +41,19 @@ fn test_hex_lowercase_uppercase() { .stdout_is("10\n"); } +#[test] +fn test_hex_big_number() { + new_ucmd!() + .args(&[ + "0xFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF", + "0x100000000000000000000000000000000", + ]) + .succeeds() + .stdout_is( + "340282366920938463463374607431768211455\n340282366920938463463374607431768211456\n", + ); +} + #[test] fn test_rejects_nan() { let ts = TestScenario::new(util_name!()); From 00c9710206a35eaf8c96b6361c2980c14697c784 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Sun, 3 Oct 2021 23:57:33 -0300 Subject: [PATCH 34/47] uucore_procs+uu: specify uucore::show_error This allows crates to use `#[uucore_procs::gen_uumain]` without `use uucore::show_error` or #[macro_use] extern crate uucore; Removed unecessary usage --- src/uu/arch/src/arch.rs | 3 --- src/uu/base32/src/base32.rs | 3 --- src/uu/base64/src/base64.rs | 3 --- src/uu/basenc/src/basenc.rs | 3 --- src/uu/cat/src/cat.rs | 2 -- src/uu/chgrp/src/chgrp.rs | 2 -- src/uu/chown/src/chown.rs | 2 -- src/uu/df/src/df.rs | 2 -- src/uu/dirname/src/dirname.rs | 3 --- src/uu/echo/src/echo.rs | 3 --- src/uu/expr/src/expr.rs | 3 --- src/uu/false/src/false.rs | 3 --- src/uu/hostid/src/hostid.rs | 3 --- src/uu/hostname/src/hostname.rs | 3 --- src/uu/mktemp/src/mktemp.rs | 3 --- src/uu/pwd/src/pwd.rs | 3 --- src/uu/sleep/src/sleep.rs | 3 --- src/uu/true/src/true.rs | 3 --- src/uu/unlink/src/unlink.rs | 3 --- src/uu/uptime/src/uptime.rs | 2 -- src/uu/whoami/src/whoami.rs | 2 -- src/uu/yes/src/yes.rs | 2 -- src/uucore_procs/src/lib.rs | 2 +- 23 files changed, 1 insertion(+), 60 deletions(-) diff --git a/src/uu/arch/src/arch.rs b/src/uu/arch/src/arch.rs index 478fef6f1..d23a11cc8 100644 --- a/src/uu/arch/src/arch.rs +++ b/src/uu/arch/src/arch.rs @@ -6,9 +6,6 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -#[macro_use] -extern crate uucore; - use platform_info::*; use clap::{crate_version, App}; diff --git a/src/uu/base32/src/base32.rs b/src/uu/base32/src/base32.rs index 0159ef181..f4b4b49de 100644 --- a/src/uu/base32/src/base32.rs +++ b/src/uu/base32/src/base32.rs @@ -5,9 +5,6 @@ // For the full copyright and license information, please view the LICENSE file // that was distributed with this source code. -#[macro_use] -extern crate uucore; - use std::io::{stdin, Read}; use clap::App; diff --git a/src/uu/base64/src/base64.rs b/src/uu/base64/src/base64.rs index ed8b1b461..c041d6d69 100644 --- a/src/uu/base64/src/base64.rs +++ b/src/uu/base64/src/base64.rs @@ -6,9 +6,6 @@ // For the full copyright and license information, please view the LICENSE file // that was distributed with this source code. -#[macro_use] -extern crate uucore; - use uu_base32::base_common; pub use uu_base32::uu_app; diff --git a/src/uu/basenc/src/basenc.rs b/src/uu/basenc/src/basenc.rs index a730d3e0f..e4b24c351 100644 --- a/src/uu/basenc/src/basenc.rs +++ b/src/uu/basenc/src/basenc.rs @@ -8,9 +8,6 @@ //spell-checker:ignore (args) lsbf msbf -#[macro_use] -extern crate uucore; - use clap::{App, Arg}; use uu_base32::base_common::{self, Config, BASE_CMD_PARSE_ERROR}; diff --git a/src/uu/cat/src/cat.rs b/src/uu/cat/src/cat.rs index baf8af6d5..308e11bfe 100644 --- a/src/uu/cat/src/cat.rs +++ b/src/uu/cat/src/cat.rs @@ -12,8 +12,6 @@ #[cfg(unix)] extern crate unix_socket; -#[macro_use] -extern crate uucore; // last synced with: cat (GNU coreutils) 8.13 use clap::{crate_version, App, Arg}; diff --git a/src/uu/chgrp/src/chgrp.rs b/src/uu/chgrp/src/chgrp.rs index 1795ad0d5..c70e5e5c7 100644 --- a/src/uu/chgrp/src/chgrp.rs +++ b/src/uu/chgrp/src/chgrp.rs @@ -7,8 +7,6 @@ // spell-checker:ignore (ToDO) COMFOLLOW Chowner RFILE RFILE's derefer dgid nonblank nonprint nonprinting -#[macro_use] -extern crate uucore; use uucore::display::Quotable; pub use uucore::entries; use uucore::error::{FromIo, UResult, USimpleError}; diff --git a/src/uu/chown/src/chown.rs b/src/uu/chown/src/chown.rs index 1cd71d3f5..f24c4ec89 100644 --- a/src/uu/chown/src/chown.rs +++ b/src/uu/chown/src/chown.rs @@ -7,8 +7,6 @@ // spell-checker:ignore (ToDO) COMFOLLOW Passwd RFILE RFILE's derefer dgid duid groupname -#[macro_use] -extern crate uucore; use uucore::display::Quotable; pub use uucore::entries::{self, Group, Locate, Passwd}; use uucore::perms::{chown_base, options, IfFrom}; diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 310d3c664..2f703542c 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -6,8 +6,6 @@ // For the full copyright and license information, please view the LICENSE file // that was distributed with this source code. -#[macro_use] -extern crate uucore; use uucore::error::UError; use uucore::error::UResult; #[cfg(unix)] diff --git a/src/uu/dirname/src/dirname.rs b/src/uu/dirname/src/dirname.rs index 601d93ac0..129c9369e 100644 --- a/src/uu/dirname/src/dirname.rs +++ b/src/uu/dirname/src/dirname.rs @@ -5,9 +5,6 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -#[macro_use] -extern crate uucore; - use clap::{crate_version, App, Arg}; use std::path::Path; use uucore::display::print_verbatim; diff --git a/src/uu/echo/src/echo.rs b/src/uu/echo/src/echo.rs index 601fd8d48..a0e6c0d9c 100644 --- a/src/uu/echo/src/echo.rs +++ b/src/uu/echo/src/echo.rs @@ -6,9 +6,6 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -#[macro_use] -extern crate uucore; - use clap::{crate_version, App, Arg}; use std::io::{self, Write}; use std::iter::Peekable; diff --git a/src/uu/expr/src/expr.rs b/src/uu/expr/src/expr.rs index eaf329bc5..6e2a8701a 100644 --- a/src/uu/expr/src/expr.rs +++ b/src/uu/expr/src/expr.rs @@ -5,9 +5,6 @@ //* For the full copyright and license information, please view the LICENSE //* file that was distributed with this source code. -#[macro_use] -extern crate uucore; - use clap::{crate_version, App, Arg}; use uucore::error::{UResult, USimpleError}; use uucore::InvalidEncodingHandling; diff --git a/src/uu/false/src/false.rs b/src/uu/false/src/false.rs index 88ec1af06..783c7fa0d 100644 --- a/src/uu/false/src/false.rs +++ b/src/uu/false/src/false.rs @@ -5,9 +5,6 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -#[macro_use] -extern crate uucore; - use clap::App; use uucore::error::UResult; diff --git a/src/uu/hostid/src/hostid.rs b/src/uu/hostid/src/hostid.rs index 4c9cafa35..309e15990 100644 --- a/src/uu/hostid/src/hostid.rs +++ b/src/uu/hostid/src/hostid.rs @@ -7,9 +7,6 @@ // spell-checker:ignore (ToDO) gethostid -#[macro_use] -extern crate uucore; - use clap::{crate_version, App}; use libc::c_long; use uucore::error::UResult; diff --git a/src/uu/hostname/src/hostname.rs b/src/uu/hostname/src/hostname.rs index 2de6627e8..9c8504027 100644 --- a/src/uu/hostname/src/hostname.rs +++ b/src/uu/hostname/src/hostname.rs @@ -7,9 +7,6 @@ // spell-checker:ignore (ToDO) MAKEWORD addrs hashset -#[macro_use] -extern crate uucore; - use std::collections::hash_set::HashSet; use std::net::ToSocketAddrs; use std::str; diff --git a/src/uu/mktemp/src/mktemp.rs b/src/uu/mktemp/src/mktemp.rs index 81a3521e9..4318f0ad6 100644 --- a/src/uu/mktemp/src/mktemp.rs +++ b/src/uu/mktemp/src/mktemp.rs @@ -8,9 +8,6 @@ // spell-checker:ignore (paths) GPGHome -#[macro_use] -extern crate uucore; - use clap::{crate_version, App, Arg}; use uucore::display::{println_verbatim, Quotable}; use uucore::error::{FromIo, UError, UResult}; diff --git a/src/uu/pwd/src/pwd.rs b/src/uu/pwd/src/pwd.rs index 1138dba8e..8beb65dbd 100644 --- a/src/uu/pwd/src/pwd.rs +++ b/src/uu/pwd/src/pwd.rs @@ -5,9 +5,6 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -#[macro_use] -extern crate uucore; - use clap::{crate_version, App, Arg}; use std::env; use std::io; diff --git a/src/uu/sleep/src/sleep.rs b/src/uu/sleep/src/sleep.rs index a70a524c4..80e8cd852 100644 --- a/src/uu/sleep/src/sleep.rs +++ b/src/uu/sleep/src/sleep.rs @@ -5,9 +5,6 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -#[macro_use] -extern crate uucore; - use std::thread; use std::time::Duration; diff --git a/src/uu/true/src/true.rs b/src/uu/true/src/true.rs index 6b4a87bf1..b5fbf7c9d 100644 --- a/src/uu/true/src/true.rs +++ b/src/uu/true/src/true.rs @@ -5,9 +5,6 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -#[macro_use] -extern crate uucore; - use clap::App; use uucore::error::UResult; diff --git a/src/uu/unlink/src/unlink.rs b/src/uu/unlink/src/unlink.rs index 1b4e4c998..58bb5442c 100644 --- a/src/uu/unlink/src/unlink.rs +++ b/src/uu/unlink/src/unlink.rs @@ -7,9 +7,6 @@ /* last synced with: unlink (GNU coreutils) 8.21 */ -#[macro_use] -extern crate uucore; - use std::fs::remove_file; use std::path::Path; diff --git a/src/uu/uptime/src/uptime.rs b/src/uu/uptime/src/uptime.rs index f649b96b6..eabcd1abf 100644 --- a/src/uu/uptime/src/uptime.rs +++ b/src/uu/uptime/src/uptime.rs @@ -11,8 +11,6 @@ use chrono::{Local, TimeZone, Utc}; use clap::{crate_version, App, Arg}; -#[macro_use] -extern crate uucore; // import crate time from utmpx pub use uucore::libc; use uucore::libc::time_t; diff --git a/src/uu/whoami/src/whoami.rs b/src/uu/whoami/src/whoami.rs index 830b86e63..0820588ee 100644 --- a/src/uu/whoami/src/whoami.rs +++ b/src/uu/whoami/src/whoami.rs @@ -9,8 +9,6 @@ #[macro_use] extern crate clap; -#[macro_use] -extern crate uucore; use clap::App; diff --git a/src/uu/yes/src/yes.rs b/src/uu/yes/src/yes.rs index 03ae4e415..bbedc0af1 100644 --- a/src/uu/yes/src/yes.rs +++ b/src/uu/yes/src/yes.rs @@ -12,8 +12,6 @@ use std::io::{self, Write}; #[macro_use] extern crate clap; -#[macro_use] -extern crate uucore; use clap::{App, Arg}; use uucore::error::{UResult, USimpleError}; diff --git a/src/uucore_procs/src/lib.rs b/src/uucore_procs/src/lib.rs index 092a4a66c..13b1dae3b 100644 --- a/src/uucore_procs/src/lib.rs +++ b/src/uucore_procs/src/lib.rs @@ -101,7 +101,7 @@ pub fn gen_uumain(_args: TokenStream, stream: TokenStream) -> TokenStream { Err(e) => { let s = format!("{}", e); if s != "" { - show_error!("{}", s); + uucore::show_error!("{}", s); } if e.usage() { eprintln!("Try '{} --help' for more information.", uucore::execution_phrase()); From e2fa6f9412a3f831c687267d9cc4f47ded80b739 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Mon, 4 Oct 2021 00:04:33 -0300 Subject: [PATCH 35/47] users: use UResult --- src/uu/users/src/users.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/uu/users/src/users.rs b/src/uu/users/src/users.rs index d374df181..d0768d8d1 100644 --- a/src/uu/users/src/users.rs +++ b/src/uu/users/src/users.rs @@ -11,6 +11,7 @@ use std::path::Path; use clap::{crate_version, App, Arg}; +use uucore::error::UResult; use uucore::utmpx::{self, Utmpx}; static ABOUT: &str = "Print the user names of users currently logged in to the current host"; @@ -29,7 +30,8 @@ If FILE is not specified, use {}. /var/log/wtmp as FILE is common.", ) } -pub fn uumain(args: impl uucore::Args) -> i32 { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let usage = usage(); let after_help = get_long_usage(); @@ -59,7 +61,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { println!("{}", users.join(" ")); } - 0 + Ok(()) } pub fn uu_app() -> App<'static, 'static> { From cddd40b4e1766a32d3f46f010e64dc07c2fb1fad Mon Sep 17 00:00:00 2001 From: vulppine Date: Tue, 5 Oct 2021 18:41:28 -0700 Subject: [PATCH 36/47] seq: Updates hex parse readability, adds hex test --- src/uu/seq/src/seq.rs | 29 +++++++++-------------------- tests/by-util/test_seq.rs | 10 ++++++++++ 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/uu/seq/src/seq.rs b/src/uu/seq/src/seq.rs index 594796641..a76a23c4e 100644 --- a/src/uu/seq/src/seq.rs +++ b/src/uu/seq/src/seq.rs @@ -89,25 +89,9 @@ impl FromStr for Number { s = &s[1..]; } let is_neg = s.starts_with('-'); - let is_hex = { - // GNU 20.11.2 - Parsing of Floats - match s.find("0x") { - Some(i) => (true, i), - None => match s.find("0X") { - Some(i) => (true, i), - None => (false, 0), - }, - } - }; - match is_hex { - (true, i) => match i <= 1 { - false => Err(format!( - "invalid hexadecimal argument: {}\nTry '{} --help' for more information.", - s.quote(), - uucore::execution_phrase(), - )), - true => match &s.as_bytes()[i + 2] { + match s.to_lowercase().find("0x") { + Some(i) if i <= 1 => match &s.as_bytes()[i + 2] { b'-' | b'+' => Err(format!( "invalid hexadecimal argument: {}\nTry '{} --help' for more information.", s.quote(), @@ -133,8 +117,13 @@ impl FromStr for Number { } } }, - }, - (false, _) => match s.parse::() { + Some(_) => Err(format!( + "invalid hexadecimal argument: {}\nTry '{} --help' for more information.", + s.quote(), + uucore::execution_phrase(), + )), + + None => match s.parse::() { Ok(n) => { // If `s` is '-0', then `parse()` returns // `BigInt::zero()`, but we need to return diff --git a/tests/by-util/test_seq.rs b/tests/by-util/test_seq.rs index 7136f5e76..6ed3cb67d 100644 --- a/tests/by-util/test_seq.rs +++ b/tests/by-util/test_seq.rs @@ -54,6 +54,16 @@ fn test_hex_big_number() { ); } +#[test] +fn test_hex_identifier_in_wrong_place() { + new_ucmd!() + .args(&["1234ABCD0x"]) + .fails() + .no_stdout() + .stderr_contains("invalid hexadecimal argument: '1234ABCD0x'") + .stderr_contains("for more information."); +} + #[test] fn test_rejects_nan() { let ts = TestScenario::new(util_name!()); From a9bfd5df0626168dd3bcbba20799182f48e4d19d Mon Sep 17 00:00:00 2001 From: Chad Brewbaker Date: Sat, 9 Oct 2021 23:01:05 -0500 Subject: [PATCH 37/47] Fix need for ssh keys Github now mandates git ssh keys instead of username/password. I modified the download instructions to remove the need for ssh authentication. --- util/build-gnu.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/build-gnu.sh b/util/build-gnu.sh index bbb16981e..eb8293fbe 100755 --- a/util/build-gnu.sh +++ b/util/build-gnu.sh @@ -5,12 +5,12 @@ set -e if test ! -d ../gnu; then echo "Could not find ../gnu" - echo "git clone git@github.com:coreutils/coreutils.git gnu" + echo "git clone https://github.com:coreutils/coreutils.git gnu" exit 1 fi if test ! -d ../gnulib; then echo "Could not find ../gnulib" - echo "git clone git@github.com:coreutils/gnulib.git gnulib" + echo "git clone https://github.com/coreutils/gnulib.git gnulib" exit 1 fi From 00de952592d97c1c717dfe769ab4d57473a1743a Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 10 Oct 2021 09:57:39 +0200 Subject: [PATCH 38/47] Fix various 'if_then_panic' clippy warnings --- tests/by-util/test_dd.rs | 8 ++--- tests/by-util/test_od.rs | 21 +++++++----- tests/common/util.rs | 72 ++++++++++++++++------------------------ 3 files changed, 42 insertions(+), 59 deletions(-) diff --git a/tests/by-util/test_dd.rs b/tests/by-util/test_dd.rs index 918be21f3..8340c7059 100644 --- a/tests/by-util/test_dd.rs +++ b/tests/by-util/test_dd.rs @@ -28,9 +28,7 @@ macro_rules! fixture_path { macro_rules! assert_fixture_exists { ($fname:expr) => {{ let fpath = fixture_path!($fname); - if !fpath.exists() { - panic!("Fixture missing: {:?}", fpath); - } + assert!(fpath.exists(), "Fixture missing: {:?}", fpath); }}; } @@ -38,9 +36,7 @@ macro_rules! assert_fixture_exists { macro_rules! assert_fixture_not_exists { ($fname:expr) => {{ let fpath = PathBuf::from(format!("./fixtures/dd/{}", $fname)); - if fpath.exists() { - panic!("Fixture present: {:?}", fpath); - } + assert!(!fpath.exists(), "Fixture present: {:?}", fpath); }}; } diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index 0fc1d5106..59a4bae60 100644 --- a/tests/by-util/test_od.rs +++ b/tests/by-util/test_od.rs @@ -35,9 +35,10 @@ fn test_file() { { let mut f = File::create(&file).unwrap(); // spell-checker:disable-next-line - if f.write_all(b"abcdefghijklmnopqrstuvwxyz\n").is_err() { - panic!("Test setup failed - could not write file"); - } + assert!( + !f.write_all(b"abcdefghijklmnopqrstuvwxyz\n").is_err(), + "Test setup failed - could not write file" + ); } new_ucmd!() @@ -75,9 +76,10 @@ fn test_2files() { // spell-checker:disable-next-line for &(path, data) in &[(&file1, "abcdefghijklmnop"), (&file2, "qrstuvwxyz\n")] { let mut f = File::create(&path).unwrap(); - if f.write_all(data.as_bytes()).is_err() { - panic!("Test setup failed - could not write file"); - } + assert!( + !f.write_all(data.as_bytes()).is_err(), + "Test setup failed - could not write file" + ); } new_ucmd!() @@ -126,9 +128,10 @@ fn test_from_mixed() { let (data1, data2, data3) = ("abcdefg", "hijklmnop", "qrstuvwxyz\n"); for &(path, data) in &[(&file1, data1), (&file3, data3)] { let mut f = File::create(&path).unwrap(); - if f.write_all(data.as_bytes()).is_err() { - panic!("Test setup failed - could not write file"); - } + assert!( + !f.write_all(data.as_bytes()).is_err(), + "Test setup failed - could not write file" + ); } new_ucmd!() diff --git a/tests/common/util.rs b/tests/common/util.rs index 8e9078e9c..61576a087 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -163,25 +163,23 @@ impl CmdResult { /// asserts that the command resulted in a success (zero) status code pub fn success(&self) -> &CmdResult { - if !self.success { - panic!( - "Command was expected to succeed.\nstdout = {}\n stderr = {}", - self.stdout_str(), - self.stderr_str() - ); - } + assert!( + self.success, + "Command was expected to succeed.\nstdout = {}\n stderr = {}", + self.stdout_str(), + self.stderr_str() + ); self } /// asserts that the command resulted in a failure (non-zero) status code pub fn failure(&self) -> &CmdResult { - if self.success { - panic!( - "Command was expected to fail.\nstdout = {}\n stderr = {}", - self.stdout_str(), - self.stderr_str() - ); - } + assert!( + !self.success, + "Command was expected to fail.\nstdout = {}\n stderr = {}", + self.stdout_str(), + self.stderr_str() + ); self } @@ -197,12 +195,11 @@ impl CmdResult { /// 1. you can not know exactly what stdout will be or /// 2. you know that stdout will also be empty pub fn no_stderr(&self) -> &CmdResult { - if !self.stderr.is_empty() { - panic!( - "Expected stderr to be empty, but it's:\n{}", - self.stderr_str() - ); - } + assert!( + self.stderr.is_empty(), + "Expected stderr to be empty, but it's:\n{}", + self.stderr_str() + ); self } @@ -213,12 +210,11 @@ impl CmdResult { /// 1. you can not know exactly what stderr will be or /// 2. you know that stderr will also be empty pub fn no_stdout(&self) -> &CmdResult { - if !self.stdout.is_empty() { - panic!( - "Expected stdout to be empty, but it's:\n{}", - self.stderr_str() - ); - } + assert!( + self.stdout.is_empty(), + "Expected stdout to be empty, but it's:\n{}", + self.stderr_str() + ); self } @@ -868,9 +864,7 @@ impl UCommand { /// Add a parameter to the invocation. Path arguments are treated relative /// to the test environment directory. pub fn arg>(&mut self, arg: S) -> &mut UCommand { - if self.has_run { - panic!("{}", ALREADY_RUN); - } + assert!(!self.has_run, ALREADY_RUN); self.comm_string.push(' '); self.comm_string .push_str(arg.as_ref().to_str().unwrap_or_default()); @@ -881,9 +875,7 @@ impl UCommand { /// Add multiple parameters to the invocation. Path arguments are treated relative /// to the test environment directory. pub fn args>(&mut self, args: &[S]) -> &mut UCommand { - if self.has_run { - panic!("{}", MULTIPLE_STDIN_MEANINGLESS); - } + assert!(!self.has_run, MULTIPLE_STDIN_MEANINGLESS); let strings = args .iter() .map(|s| s.as_ref().to_os_string()) @@ -901,9 +893,7 @@ impl UCommand { /// provides standard input to feed in to the command when spawned pub fn pipe_in>>(&mut self, input: T) -> &mut UCommand { - if self.bytes_into_stdin.is_some() { - panic!("{}", MULTIPLE_STDIN_MEANINGLESS); - } + assert!(!self.bytes_into_stdin.is_some(), MULTIPLE_STDIN_MEANINGLESS); self.bytes_into_stdin = Some(input.into()); self } @@ -918,9 +908,7 @@ impl UCommand { /// This is typically useful to test non-standard workflows /// like feeding something to a command that does not read it pub fn ignore_stdin_write_error(&mut self) -> &mut UCommand { - if self.bytes_into_stdin.is_none() { - panic!("{}", NO_STDIN_MEANINGLESS); - } + assert!(!self.bytes_into_stdin.is_none(), NO_STDIN_MEANINGLESS); self.ignore_stdin_write_error = true; self } @@ -930,9 +918,7 @@ impl UCommand { K: AsRef, V: AsRef, { - if self.has_run { - panic!("{}", ALREADY_RUN); - } + assert!(!self.has_run, ALREADY_RUN); self.raw.env(key, val); self } @@ -951,9 +937,7 @@ impl UCommand { /// Spawns the command, feeds the stdin if any, and returns the /// child process immediately. pub fn run_no_wait(&mut self) -> Child { - if self.has_run { - panic!("{}", ALREADY_RUN); - } + assert!(!self.has_run, ALREADY_RUN); self.has_run = true; log_info("run", &self.comm_string); let mut child = self From b4864d760eb51230b6b4517e93f76298bcaa0f58 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Sun, 10 Oct 2021 11:33:46 +0200 Subject: [PATCH 39/47] pr: fix locking of stdout --- src/uu/pr/src/pr.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/uu/pr/src/pr.rs b/src/uu/pr/src/pr.rs index 45d9480a7..0886a9991 100644 --- a/src/uu/pr/src/pr.rs +++ b/src/uu/pr/src/pr.rs @@ -20,7 +20,7 @@ use quick_error::ResultExt; use regex::Regex; use std::convert::From; use std::fs::{metadata, File}; -use std::io::{stdin, stdout, BufRead, BufReader, Lines, Read, Stdout, Write}; +use std::io::{stdin, stdout, BufRead, BufReader, Lines, Read, Write}; #[cfg(unix)] use std::os::unix::fs::FileTypeExt; @@ -1036,15 +1036,16 @@ fn print_page(lines: &[FileLine], options: &OutputOptions, page: usize) -> Resul let header = header_content(options, page); let trailer_content = trailer_content(options); - let out = &mut stdout(); - out.lock(); + let out = stdout(); + let mut out = out.lock(); + for x in header { out.write_all(x.as_bytes())?; out.write_all(line_separator)?; } - let lines_written = write_columns(lines, options, out)?; + let lines_written = write_columns(lines, options, &mut out)?; for (index, x) in trailer_content.iter().enumerate() { out.write_all(x.as_bytes())?; @@ -1060,7 +1061,7 @@ fn print_page(lines: &[FileLine], options: &OutputOptions, page: usize) -> Resul fn write_columns( lines: &[FileLine], options: &OutputOptions, - out: &mut Stdout, + out: &mut impl Write, ) -> Result { let line_separator = options.content_line_separator.as_bytes(); From 51613c02eca5db5e935232b61b1a353e6f13966b Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sun, 10 Oct 2021 10:10:46 +0200 Subject: [PATCH 40/47] add a word to ignore... --- tests/by-util/test_od.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/by-util/test_od.rs b/tests/by-util/test_od.rs index 59a4bae60..6c167b325 100644 --- a/tests/by-util/test_od.rs +++ b/tests/by-util/test_od.rs @@ -1,3 +1,4 @@ +// spell-checker:ignore abcdefghijklmnopqrstuvwxyz // * This file is part of the uutils coreutils package. // * // * For the full copyright and license information, please view the LICENSE From e041fda51d59a63783648815f345830f1d0163d9 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Wed, 6 Oct 2021 20:33:13 +0200 Subject: [PATCH 41/47] tac: do not re-compile regular expression for each file --- src/uu/tac/src/tac.rs | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index 4a93a7c65..caf77c4a7 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -44,9 +44,9 @@ pub fn uumain(args: impl uucore::Args) -> i32 { raw_separator }; - let files: Vec = match matches.values_of(options::FILE) { - Some(v) => v.map(|v| v.to_owned()).collect(), - None => vec!["-".to_owned()], + let files: Vec<&str> = match matches.values_of(options::FILE) { + Some(v) => v.collect(), + None => vec!["-"], }; tac(files, before, regex, separator) @@ -102,7 +102,7 @@ pub fn uu_app() -> App<'static, 'static> { /// returns [`std::io::Error`]. fn buffer_tac_regex( data: &[u8], - pattern: regex::bytes::Regex, + pattern: ®ex::bytes::Regex, before: bool, ) -> std::io::Result<()> { let mut out = stdout(); @@ -208,10 +208,16 @@ fn buffer_tac(data: &[u8], before: bool, separator: &str) -> std::io::Result<()> Ok(()) } -fn tac(filenames: Vec, before: bool, regex: bool, separator: &str) -> i32 { +fn tac(filenames: Vec<&str>, before: bool, regex: bool, separator: &str) -> i32 { let mut exit_code = 0; - for filename in &filenames { + let pattern = if regex { + Some(crash_if_err!(1, regex::bytes::Regex::new(separator))) + } else { + None + }; + + for &filename in &filenames { let mut file = BufReader::new(if filename == "-" { Box::new(stdin()) as Box } else { @@ -244,8 +250,7 @@ fn tac(filenames: Vec, before: bool, regex: bool, separator: &str) -> i3 exit_code = 1; continue; }; - if regex { - let pattern = crash_if_err!(1, regex::bytes::Regex::new(separator)); + if let Some(pattern) = &pattern { buffer_tac_regex(&data, pattern, before) } else { buffer_tac(&data, before, separator) From 0d583754cad9f4d6587469a428fb41a69059818c Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Wed, 6 Oct 2021 21:38:08 +0200 Subject: [PATCH 42/47] tac: lock stdout only once instead for each line --- src/uu/tac/src/tac.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index caf77c4a7..eab6a943b 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -105,7 +105,8 @@ fn buffer_tac_regex( pattern: ®ex::bytes::Regex, before: bool, ) -> std::io::Result<()> { - let mut out = stdout(); + let out = stdout(); + let mut out = out.lock(); // The index of the line separator for the current line. // @@ -171,7 +172,8 @@ fn buffer_tac_regex( /// `separator` appears at the beginning of each line, as in /// `"/abc/def"`. fn buffer_tac(data: &[u8], before: bool, separator: &str) -> std::io::Result<()> { - let mut out = stdout(); + let out = stdout(); + let mut out = out.lock(); // The number of bytes in the line separator. let slen = separator.as_bytes().len(); From 28b04fa89950ff1bac7863934050ef9c26a0022d Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Wed, 6 Oct 2021 20:50:34 +0200 Subject: [PATCH 43/47] tac: do not use a buffered read as fs::read is more efficient and io::Stdin is buffered internally --- src/uu/tac/src/tac.rs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index eab6a943b..92bf1ea00 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -12,8 +12,8 @@ extern crate uucore; use clap::{crate_version, App, Arg}; use memchr::memmem; -use std::io::{stdin, stdout, BufReader, Read, Write}; -use std::{fs::File, path::Path}; +use std::io::{stdin, stdout, Read, Write}; +use std::{fs::read, path::Path}; use uucore::display::Quotable; use uucore::InvalidEncodingHandling; @@ -220,8 +220,14 @@ fn tac(filenames: Vec<&str>, before: bool, regex: bool, separator: &str) -> i32 }; for &filename in &filenames { - let mut file = BufReader::new(if filename == "-" { - Box::new(stdin()) as Box + let data = if filename == "-" { + let mut data = Vec::new(); + if let Err(e) = stdin().read_to_end(&mut data) { + show_error!("failed to read from stdin: {}", e); + exit_code = 1; + continue; + } + data } else { let path = Path::new(filename); if path.is_dir() || path.metadata().is_err() { @@ -236,22 +242,16 @@ fn tac(filenames: Vec<&str>, before: bool, regex: bool, separator: &str) -> i32 exit_code = 1; continue; } - match File::open(path) { - Ok(f) => Box::new(f) as Box, + match read(path) { + Ok(data) => data, Err(e) => { - show_error!("failed to open {} for reading: {}", filename.quote(), e); + show_error!("failed to read {}: {}", filename.quote(), e); exit_code = 1; continue; } } - }); - - let mut data = Vec::new(); - if let Err(e) = file.read_to_end(&mut data) { - show_error!("failed to read {}: {}", filename.quote(), e); - exit_code = 1; - continue; }; + if let Some(pattern) = &pattern { buffer_tac_regex(&data, pattern, before) } else { From 4eab275235f2d89ea48bb7b2b921ea33a8db3fa7 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Sat, 9 Oct 2021 12:12:57 +0200 Subject: [PATCH 44/47] tac: buffer stdout more coarsely than line-based following the GNU tac implementation. --- src/uu/tac/src/tac.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index 92bf1ea00..370e92230 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -12,7 +12,7 @@ extern crate uucore; use clap::{crate_version, App, Arg}; use memchr::memmem; -use std::io::{stdin, stdout, Read, Write}; +use std::io::{stdin, stdout, BufWriter, Read, Write}; use std::{fs::read, path::Path}; use uucore::display::Quotable; use uucore::InvalidEncodingHandling; @@ -106,7 +106,7 @@ fn buffer_tac_regex( before: bool, ) -> std::io::Result<()> { let out = stdout(); - let mut out = out.lock(); + let mut out = BufWriter::new(out.lock()); // The index of the line separator for the current line. // @@ -173,7 +173,7 @@ fn buffer_tac_regex( /// `"/abc/def"`. fn buffer_tac(data: &[u8], before: bool, separator: &str) -> std::io::Result<()> { let out = stdout(); - let mut out = out.lock(); + let mut out = BufWriter::new(out.lock()); // The number of bytes in the line separator. let slen = separator.as_bytes().len(); From c526df57b8e10b94b096476c5eaf20eae52e7061 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Wed, 6 Oct 2021 21:08:11 +0200 Subject: [PATCH 45/47] tac: opportunistically use memory maps Since tac must read its input files completely to start processing them from the end, it is particularly suited to use memory maps to benefit from the page cache maintained by the operating systems to bring the necessary data into memory as required. This does also include situations where the input is stdin, but not via a pipe but for example a file descriptor set up by the user's shell through an input redirection. --- Cargo.lock | 10 ++++++ src/uu/tac/Cargo.toml | 3 ++ src/uu/tac/src/tac.rs | 72 +++++++++++++++++++++++++++++++++---------- 3 files changed, 68 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6c30a182d..3633928c6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1064,6 +1064,15 @@ version = "2.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b16bd47d9e329435e309c58469fe0791c2d0d1ba96ec0954152a5ae2b04387dc" +[[package]] +name = "memmap2" +version = "0.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4647a11b578fead29cdbb34d4adef8dd3dc35b876c9c6d5240d83f205abfe96e" +dependencies = [ + "libc", +] + [[package]] name = "memoffset" version = "0.6.4" @@ -3025,6 +3034,7 @@ version = "0.0.7" dependencies = [ "clap", "memchr 2.4.0", + "memmap2", "regex", "uucore", "uucore_procs", diff --git a/src/uu/tac/Cargo.toml b/src/uu/tac/Cargo.toml index 1e436e916..00803c8d2 100644 --- a/src/uu/tac/Cargo.toml +++ b/src/uu/tac/Cargo.toml @@ -1,3 +1,5 @@ +# spell-checker:ignore memmap + [package] name = "uu_tac" version = "0.0.7" @@ -16,6 +18,7 @@ path = "src/tac.rs" [dependencies] memchr = "2" +memmap2 = "0.5" regex = "1" clap = { version = "2.33", features = ["wrap_help"] } uucore = { version=">=0.0.9", package="uucore", path="../../uucore" } diff --git a/src/uu/tac/src/tac.rs b/src/uu/tac/src/tac.rs index 370e92230..cdb2d74e3 100644 --- a/src/uu/tac/src/tac.rs +++ b/src/uu/tac/src/tac.rs @@ -5,15 +5,19 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -// spell-checker:ignore (ToDO) sbytes slen dlen memmem +// spell-checker:ignore (ToDO) sbytes slen dlen memmem memmap Mmap mmap SIGBUS #[macro_use] extern crate uucore; use clap::{crate_version, App, Arg}; use memchr::memmem; +use memmap2::Mmap; use std::io::{stdin, stdout, BufWriter, Read, Write}; -use std::{fs::read, path::Path}; +use std::{ + fs::{read, File}, + path::Path, +}; use uucore::display::Quotable; use uucore::InvalidEncodingHandling; @@ -220,14 +224,23 @@ fn tac(filenames: Vec<&str>, before: bool, regex: bool, separator: &str) -> i32 }; for &filename in &filenames { - let data = if filename == "-" { - let mut data = Vec::new(); - if let Err(e) = stdin().read_to_end(&mut data) { - show_error!("failed to read from stdin: {}", e); - exit_code = 1; - continue; + let mmap; + let buf; + + let data: &[u8] = if filename == "-" { + if let Some(mmap1) = try_mmap_stdin() { + mmap = mmap1; + &mmap + } else { + let mut buf1 = Vec::new(); + if let Err(e) = stdin().read_to_end(&mut buf1) { + show_error!("failed to read from stdin: {}", e); + exit_code = 1; + continue; + } + buf = buf1; + &buf } - data } else { let path = Path::new(filename); if path.is_dir() || path.metadata().is_err() { @@ -242,22 +255,47 @@ fn tac(filenames: Vec<&str>, before: bool, regex: bool, separator: &str) -> i32 exit_code = 1; continue; } - match read(path) { - Ok(data) => data, - Err(e) => { - show_error!("failed to read {}: {}", filename.quote(), e); - exit_code = 1; - continue; + + if let Some(mmap1) = try_mmap_path(path) { + mmap = mmap1; + &mmap + } else { + match read(path) { + Ok(buf1) => { + buf = buf1; + &buf + } + Err(e) => { + show_error!("failed to read {}: {}", filename.quote(), e); + exit_code = 1; + continue; + } } } }; if let Some(pattern) = &pattern { - buffer_tac_regex(&data, pattern, before) + buffer_tac_regex(data, pattern, before) } else { - buffer_tac(&data, before, separator) + buffer_tac(data, before, separator) } .unwrap_or_else(|e| crash!(1, "failed to write to stdout: {}", e)); } exit_code } + +fn try_mmap_stdin() -> Option { + // SAFETY: If the file is truncated while we map it, SIGBUS will be raised + // and our process will be terminated, thus preventing access of invalid memory. + unsafe { Mmap::map(&stdin()).ok() } +} + +fn try_mmap_path(path: &Path) -> Option { + let file = File::open(path).ok()?; + + // SAFETY: If the file is truncated while we map it, SIGBUS will be raised + // and our process will be terminated, thus preventing access of invalid memory. + let mmap = unsafe { Mmap::map(&file).ok()? }; + + Some(mmap) +} From 86d22aaa1d4a1bd252083cfd2a144bbfa5198c8f Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Sun, 10 Oct 2021 13:54:35 +0200 Subject: [PATCH 46/47] tac: Add a simple how to for benchmarking --- src/uu/tac/BENCHMARKING.md | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 src/uu/tac/BENCHMARKING.md diff --git a/src/uu/tac/BENCHMARKING.md b/src/uu/tac/BENCHMARKING.md new file mode 100644 index 000000000..4e6d13ea2 --- /dev/null +++ b/src/uu/tac/BENCHMARKING.md @@ -0,0 +1,25 @@ +## Benchmarking `tac` + + + +`tac` is often used to process log files in reverse chronological order, i.e. from newer towards older entries. In this case, the performance target to yield results as fast as possible, i.e. without reading in the whole file that is to be reversed line-by-line. Therefore, a sensible benchmark is to read a large log file containing N lines and measure how long it takes to produce the last K lines from that file. + +Large text files can for example be found in the [Wikipedia database dumps](https://dumps.wikimedia.org/wikidatawiki/latest/), usually sized at multiple gigabytes and comprising more than 100M lines. + +After you have obtained and uncompressed such a file, you need to build `tac` in release mode + +```shell +$ cargo build --release --package uu_tac +``` + +and then you can time how it long it takes to extract the last 10M lines by running + +```shell +$ /usr/bin/time ./target/release/tac wikidatawiki-20211001-pages-logging.xml | head -n10000000 >/dev/null +``` + +For more systematic measurements that include warm-ups, repetitions and comparisons, [Hyperfine](https://github.com/sharkdp/hyperfine) can be helpful. For example, to compare this implementation to the one provided by your distribution run + +```shell +$ hyperfine "./target/release/tac wikidatawiki-20211001-pages-logging.xml | head -n10000000 >/dev/null" "/usr/bin/tac wikidatawiki-20211001-pages-logging.xml | head -n10000000 >/dev/null" +``` From 429e1d0f12eadf6a709bbdbf55aa2c0ce70657df Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Mon, 11 Oct 2021 18:29:08 -0400 Subject: [PATCH 47/47] head: use default() instead of new() for options Remove the `HeadOptions::new()` function in favor of the `Default` implementation. Both were implemented, but only `Default::default()` is needed. --- src/uu/head/src/head.rs | 41 +++++++++++++---------------------------- 1 file changed, 13 insertions(+), 28 deletions(-) diff --git a/src/uu/head/src/head.rs b/src/uu/head/src/head.rs index ead734088..c6635ac67 100644 --- a/src/uu/head/src/head.rs +++ b/src/uu/head/src/head.rs @@ -108,6 +108,12 @@ enum Modes { Bytes(usize), } +impl Default for Modes { + fn default() -> Self { + Modes::Lines(10) + } +} + fn parse_mode(src: &str, closure: F) -> Result<(Modes, bool), String> where F: FnOnce(usize) -> Modes, @@ -144,7 +150,7 @@ fn arg_iterate<'a>( } } -#[derive(Debug, PartialEq)] +#[derive(Debug, PartialEq, Default)] struct HeadOptions { pub quiet: bool, pub verbose: bool, @@ -155,22 +161,11 @@ struct HeadOptions { } impl HeadOptions { - pub fn new() -> HeadOptions { - HeadOptions { - quiet: false, - verbose: false, - zeroed: false, - all_but_last: false, - mode: Modes::Lines(10), - files: Vec::new(), - } - } - ///Construct options from matches pub fn get_from(args: impl uucore::Args) -> Result { let matches = uu_app().get_matches_from(arg_iterate(args)?); - let mut options = HeadOptions::new(); + let mut options: HeadOptions = Default::default(); options.quiet = matches.is_present(options::QUIET_NAME); options.verbose = matches.is_present(options::VERBOSE_NAME); @@ -197,12 +192,6 @@ impl HeadOptions { Ok(options) } } -// to make clippy shut up -impl Default for HeadOptions { - fn default() -> Self { - Self::new() - } -} fn read_n_bytes(input: R, n: usize) -> std::io::Result<()> where @@ -523,17 +512,13 @@ mod tests { assert!(options("-c IsThisJustFantasy").is_err()); } #[test] - #[allow(clippy::bool_comparison)] fn test_options_correct_defaults() { - let opts = HeadOptions::new(); - let opts2: HeadOptions = Default::default(); + let opts: HeadOptions = Default::default(); - assert_eq!(opts, opts2); - - assert!(opts.verbose == false); - assert!(opts.quiet == false); - assert!(opts.zeroed == false); - assert!(opts.all_but_last == false); + assert!(!opts.verbose); + assert!(!opts.quiet); + assert!(!opts.zeroed); + assert!(!opts.all_but_last); assert_eq!(opts.mode, Modes::Lines(10)); assert!(opts.files.is_empty()); }