From e1d50dae87036186e9b4e1f21042218e2eb813eb Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Fri, 6 May 2022 13:58:05 +0200 Subject: [PATCH 01/11] kill: fix typo --- src/uu/kill/src/kill.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/uu/kill/src/kill.rs b/src/uu/kill/src/kill.rs index df868e418..e77d0c666 100644 --- a/src/uu/kill/src/kill.rs +++ b/src/uu/kill/src/kill.rs @@ -22,7 +22,7 @@ static ABOUT: &str = "Send signal to processes or list information about signals const USAGE: &str = "{} [OPTIONS]... PID..."; pub mod options { - pub static PIDS_OR_SIGNALS: &str = "pids_of_signals"; + pub static PIDS_OR_SIGNALS: &str = "pids_or_signals"; pub static LIST: &str = "list"; pub static TABLE: &str = "table"; pub static TABLE_OLD: &str = "table_old"; @@ -109,7 +109,8 @@ pub fn uu_app<'a>() -> Command<'a> { .arg( Arg::new(options::PIDS_OR_SIGNALS) .hide(true) - .multiple_occurrences(true), + .multiple_occurrences(true) + .allow_hyphen_values(true) ) } From a7cf757127c38e9e8b4d9892419110f75c48d9da Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Fri, 6 May 2022 23:55:54 +0200 Subject: [PATCH 02/11] kill: kill process group with negative id --- Cargo.lock | 3 ++- src/uu/kill/Cargo.toml | 2 +- src/uu/kill/src/kill.rs | 29 +++++++++++++++++------------ 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1079f8a65..4c858b83f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1158,6 +1158,7 @@ dependencies = [ "bitflags", "cfg-if 1.0.0", "libc", + "memoffset", ] [[package]] @@ -2452,7 +2453,7 @@ name = "uu_kill" version = "0.0.13" dependencies = [ "clap 3.1.15", - "libc", + "nix", "uucore", ] diff --git a/src/uu/kill/Cargo.toml b/src/uu/kill/Cargo.toml index 24347a90a..4ab76deea 100644 --- a/src/uu/kill/Cargo.toml +++ b/src/uu/kill/Cargo.toml @@ -16,7 +16,7 @@ path = "src/kill.rs" [dependencies] clap = { version = "3.1", features = ["wrap_help", "cargo"] } -libc = "0.2.125" +nix = { version = "0.24.1", features = ["signal"] } uucore = { version=">=0.0.11", package="uucore", path="../../uucore", features=["signals"] } [[bin]] diff --git a/src/uu/kill/src/kill.rs b/src/uu/kill/src/kill.rs index e77d0c666..c966c7fac 100644 --- a/src/uu/kill/src/kill.rs +++ b/src/uu/kill/src/kill.rs @@ -5,16 +5,17 @@ // * For the full copyright and license information, please view the LICENSE file // * that was distributed with this source code. -// spell-checker:ignore (ToDO) signalname pids +// spell-checker:ignore (ToDO) signalname pids killpg #[macro_use] extern crate uucore; use clap::{crate_version, Arg, Command}; -use libc::{c_int, pid_t}; +use nix::sys::signal::{self, Signal}; +use nix::unistd::Pid; use std::io::Error; use uucore::display::Quotable; -use uucore::error::{UResult, USimpleError}; +use uucore::error::{FromIo, UError, UResult, USimpleError}; use uucore::signals::{signal_by_name_or_value, ALL_SIGNALS}; use uucore::{format_usage, InvalidEncodingHandling}; @@ -67,8 +68,12 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { } else { 15_usize //SIGTERM }; + let sig: Signal = (sig as i32) + .try_into() + .map_err(|e| std::io::Error::from_raw_os_error(e as i32))?; let pids = parse_pids(&pids_or_signals)?; - kill(sig, &pids) + kill(sig, &pids); + Ok(()) } Mode::Table => { table(); @@ -84,6 +89,7 @@ pub fn uu_app<'a>() -> Command<'a> { .about(ABOUT) .override_usage(format_usage(USAGE)) .infer_long_args(true) + .allow_negative_numbers(true) .arg( Arg::new(options::LIST) .short('l') @@ -109,8 +115,7 @@ pub fn uu_app<'a>() -> Command<'a> { .arg( Arg::new(options::PIDS_OR_SIGNALS) .hide(true) - .multiple_occurrences(true) - .allow_hyphen_values(true) + .multiple_occurrences(true), ) } @@ -191,21 +196,21 @@ fn parse_signal_value(signal_name: &str) -> UResult { } } -fn parse_pids(pids: &[String]) -> UResult> { +fn parse_pids(pids: &[String]) -> UResult> { pids.iter() .map(|x| { - x.parse::().map_err(|e| { + x.parse::().map_err(|e| { USimpleError::new(1, format!("failed to parse argument {}: {}", x.quote(), e)) }) }) .collect() } -fn kill(signal_value: usize, pids: &[usize]) -> UResult<()> { +fn kill(sig: Signal, pids: &[i32]) { for &pid in pids { - if unsafe { libc::kill(pid as pid_t, signal_value as c_int) } != 0 { - show!(USimpleError::new(1, format!("{}", Error::last_os_error()))); + if let Err(e) = signal::kill(Pid::from_raw(pid), sig) { + show!(Error::from_raw_os_error(e as i32) + .map_err_context(|| format!("sending signal to {} failed", pid))); } } - Ok(()) } From 53c3efecd898d04623fb374ddfcdd19ac76197bd Mon Sep 17 00:00:00 2001 From: Terts Diepraam Date: Sat, 7 May 2022 12:08:06 +0200 Subject: [PATCH 03/11] kill: remove table_old arg in favor of a short alias --- src/uu/kill/src/kill.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/uu/kill/src/kill.rs b/src/uu/kill/src/kill.rs index c966c7fac..5ae836818 100644 --- a/src/uu/kill/src/kill.rs +++ b/src/uu/kill/src/kill.rs @@ -26,7 +26,6 @@ pub mod options { pub static PIDS_OR_SIGNALS: &str = "pids_or_signals"; pub static LIST: &str = "list"; pub static TABLE: &str = "table"; - pub static TABLE_OLD: &str = "table_old"; pub static SIGNAL: &str = "signal"; } @@ -46,7 +45,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().get_matches_from(args); - let mode = if matches.is_present(options::TABLE) || matches.is_present(options::TABLE_OLD) { + let mode = if matches.is_present(options::TABLE) { Mode::Table } else if matches.is_present(options::LIST) { Mode::List @@ -95,16 +94,15 @@ pub fn uu_app<'a>() -> Command<'a> { .short('l') .long(options::LIST) .help("Lists signals") - .conflicts_with(options::TABLE) - .conflicts_with(options::TABLE_OLD), + .conflicts_with(options::TABLE), ) .arg( Arg::new(options::TABLE) .short('t') + .short_alias('L') .long(options::TABLE) .help("Lists table of signals"), ) - .arg(Arg::new(options::TABLE_OLD).short('L').hide(true)) .arg( Arg::new(options::SIGNAL) .short('s') From 530d5f6dbfc1edaa5efaa85884db8749ffabd876 Mon Sep 17 00:00:00 2001 From: ilkecan Date: Sat, 14 May 2022 03:00:29 +0000 Subject: [PATCH 04/11] mv: allow a single source with --target-directory --- src/uu/mv/src/mv.rs | 31 ++++++++++++++++++++++--------- tests/by-util/test_mv.rs | 14 ++++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/uu/mv/src/mv.rs b/src/uu/mv/src/mv.rs index 1c2390f80..b6a6712de 100644 --- a/src/uu/mv/src/mv.rs +++ b/src/uu/mv/src/mv.rs @@ -13,7 +13,7 @@ mod error; #[macro_use] extern crate uucore; -use clap::{crate_version, Arg, ArgMatches, Command}; +use clap::{crate_version, Arg, ArgMatches, Command, ErrorKind}; use std::env; use std::ffi::OsString; use std::fs; @@ -70,13 +70,26 @@ static ARG_FILES: &str = "files"; #[uucore::main] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let matches = uu_app() - .after_help(&*format!( - "{}\n{}", - LONG_HELP, - backup_control::BACKUP_CONTROL_LONG_HELP - )) - .get_matches_from(args); + let help = format!( + "{}\n{}", + LONG_HELP, + backup_control::BACKUP_CONTROL_LONG_HELP + ); + let mut app = uu_app().after_help(&*help); + let matches = app + .try_get_matches_from_mut(args) + .unwrap_or_else(|e| e.exit()); + + if !matches.is_present(OPT_TARGET_DIRECTORY) && matches.occurrences_of(ARG_FILES) == 1 { + app.error( + ErrorKind::TooFewValues, + format!( + "The argument '<{}>...' requires at least 2 values, but only 1 was provided", + ARG_FILES + ), + ) + .exit(); + } let files: Vec = matches .values_of_os(ARG_FILES) @@ -180,7 +193,7 @@ pub fn uu_app<'a>() -> Command<'a> { Arg::new(ARG_FILES) .multiple_occurrences(true) .takes_value(true) - .min_values(2) + .min_values(1) .required(true) .allow_invalid_utf8(true) ) diff --git a/tests/by-util/test_mv.rs b/tests/by-util/test_mv.rs index 06f4d5259..96bd96856 100644 --- a/tests/by-util/test_mv.rs +++ b/tests/by-util/test_mv.rs @@ -641,6 +641,20 @@ fn test_mv_target_dir() { assert!(at.file_exists(&format!("{}/{}", dir, file_b))); } +#[test] +fn test_mv_target_dir_single_source() { + let (at, mut ucmd) = at_and_ucmd!(); + let dir = "test_mv_target_dir_single_source_dir"; + let file = "test_mv_target_dir_single_source_file"; + + at.touch(file); + at.mkdir(dir); + ucmd.arg("-t").arg(dir).arg(file).succeeds().no_stderr(); + + assert!(!at.file_exists(file)); + assert!(at.file_exists(&format!("{}/{}", dir, file))); +} + #[test] fn test_mv_overwrite_dir() { let (at, mut ucmd) = at_and_ucmd!(); From 576aafb00f5e8cfd9d294578412b594734f84820 Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Sun, 15 May 2022 16:25:17 +0200 Subject: [PATCH 05/11] df: fix incorrect rounding of size header --- src/uu/df/src/blocks.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/uu/df/src/blocks.rs b/src/uu/df/src/blocks.rs index 594c18acc..88190b5c1 100644 --- a/src/uu/df/src/blocks.rs +++ b/src/uu/df/src/blocks.rs @@ -99,7 +99,7 @@ fn to_magnitude_and_suffix_not_powers_of_1024(n: u128) -> Result { if rem % (SI_BASES[i] / 10) == 0 { Ok(format!("{}.{}{}", quot, tenths_place, suffix)) - } else if tenths_place + 1 == 10 { + } else if tenths_place + 1 == 10 || quot >= 10 { Ok(format!("{}{}", quot + 1, suffix)) } else { Ok(format!("{}.{}{}", quot, tenths_place + 1, suffix)) @@ -245,6 +245,7 @@ mod tests { assert_eq!(to_magnitude_and_suffix(1001).unwrap(), "1.1kB"); assert_eq!(to_magnitude_and_suffix(1023).unwrap(), "1.1kB"); assert_eq!(to_magnitude_and_suffix(1025).unwrap(), "1.1kB"); + assert_eq!(to_magnitude_and_suffix(10_001).unwrap(), "11kB"); assert_eq!(to_magnitude_and_suffix(999_000).unwrap(), "999kB"); assert_eq!(to_magnitude_and_suffix(999_001).unwrap(), "1MB"); From 8a941db20ab44d4bb3ea4112b297ca2266e84bf7 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Wed, 11 May 2022 22:31:09 -0400 Subject: [PATCH 06/11] mktemp: correct error message on absolute path Correct the error message produced by `mktemp` when `--tmpdir` is given and the template is an absolute path: $ mktemp --tmpdir=a /XXX mktemp: invalid template, '/XXX'; with --tmpdir, it may not be absolute --- src/uu/mktemp/src/mktemp.rs | 8 +++++--- tests/by-util/test_mktemp.rs | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 3 deletions(-) diff --git a/src/uu/mktemp/src/mktemp.rs b/src/uu/mktemp/src/mktemp.rs index 20b95f009..541b1310e 100644 --- a/src/uu/mktemp/src/mktemp.rs +++ b/src/uu/mktemp/src/mktemp.rs @@ -139,12 +139,14 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let dry_run = matches.is_present(OPT_DRY_RUN); let suppress_file_err = matches.is_present(OPT_QUIET); - let (prefix, rand, suffix) = parse_template(template, matches.value_of(OPT_SUFFIX))?; - - if matches.is_present(OPT_TMPDIR) && PathBuf::from(prefix).is_absolute() { + // If `--tmpdir` is given, the template cannot be an absolute + // path. For example, `mktemp --tmpdir=a /XXX` is not allowed. + if matches.is_present(OPT_TMPDIR) && PathBuf::from(template).is_absolute() { return Err(MkTempError::InvalidTemplate(template.into()).into()); } + let (prefix, rand, suffix) = parse_template(template, matches.value_of(OPT_SUFFIX))?; + let res = if dry_run { dry_exec(tmpdir, prefix, rand, suffix) } else { diff --git a/tests/by-util/test_mktemp.rs b/tests/by-util/test_mktemp.rs index 9fce0e591..8cdddd82c 100644 --- a/tests/by-util/test_mktemp.rs +++ b/tests/by-util/test_mktemp.rs @@ -414,6 +414,22 @@ fn test_mktemp_directory_tmpdir() { assert!(PathBuf::from(result.stdout_str().trim()).is_dir()); } +/// Test that an absolute path is disallowed when --tmpdir is provided. +#[test] +fn test_tmpdir_absolute_path() { + #[cfg(windows)] + let path = r"C:\XXX"; + #[cfg(not(windows))] + let path = "/XXX"; + new_ucmd!() + .args(&["--tmpdir=a", path]) + .fails() + .stderr_only(format!( + "mktemp: invalid template, '{}'; with --tmpdir, it may not be absolute\n", + path + )); +} + /// Decide whether a string matches a given template. /// /// In the template, the character `'X'` is treated as a wildcard, From a3e6d2b84b97d8493a6be650a55f307334836d52 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Tue, 17 May 2022 21:02:42 -0400 Subject: [PATCH 07/11] seq: use usage error where appropriate --- src/uu/seq/src/error.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/uu/seq/src/error.rs b/src/uu/seq/src/error.rs index 8c372758c..ae641a978 100644 --- a/src/uu/seq/src/error.rs +++ b/src/uu/seq/src/error.rs @@ -31,20 +31,20 @@ impl SeqError { /// The [`String`] argument as read from the command-line. fn arg(&self) -> &str { match self { - SeqError::ParseError(s, _) => s, - SeqError::ZeroIncrement(s) => s, + Self::ParseError(s, _) => s, + Self::ZeroIncrement(s) => s, } } /// The type of argument that is causing the error. fn argtype(&self) -> &str { match self { - SeqError::ParseError(_, e) => match e { + Self::ParseError(_, e) => match e { ParseNumberError::Float => "floating point argument", ParseNumberError::Nan => "'not-a-number' argument", ParseNumberError::Hex => "hexadecimal argument", }, - SeqError::ZeroIncrement(_) => "Zero increment value", + Self::ZeroIncrement(_) => "Zero increment value", } } } @@ -53,18 +53,16 @@ impl UError for SeqError { fn code(&self) -> i32 { 1 } + + fn usage(&self) -> bool { + true + } } impl Error for SeqError {} impl Display for SeqError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "invalid {}: {}\nTry '{} --help' for more information.", - self.argtype(), - self.arg().quote(), - uucore::execution_phrase(), - ) + write!(f, "invalid {}: {}", self.argtype(), self.arg().quote()) } } From 6260333415cba80b59936257f0995f5ee9553c3f Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Wed, 18 May 2022 18:33:53 -0400 Subject: [PATCH 08/11] mktemp: fix error msg when suffix has path sep. Correct the error message when the template argument contains a path separator in its suffix. Before this commit: $ mktemp aXXX/b mktemp: too few X's in template 'b' After this commit: $ mktemp aXXX/b mktemp: invalid suffix '/b', contains directory separator This error message is more appropriate and matches the behavior of GNU mktemp. --- src/uu/mktemp/src/mktemp.rs | 15 ++++++++++++++- tests/by-util/test_mktemp.rs | 15 +++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/src/uu/mktemp/src/mktemp.rs b/src/uu/mktemp/src/mktemp.rs index 20b95f009..8cefc7982 100644 --- a/src/uu/mktemp/src/mktemp.rs +++ b/src/uu/mktemp/src/mktemp.rs @@ -17,7 +17,7 @@ use std::env; use std::error::Error; use std::fmt::Display; use std::iter; -use std::path::{is_separator, Path, PathBuf}; +use std::path::{is_separator, Path, PathBuf, MAIN_SEPARATOR}; use rand::Rng; use tempfile::Builder; @@ -129,6 +129,19 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }; let filename = path.file_name(); let template = filename.unwrap().to_str().unwrap(); + // If the command line was `mktemp aXXX/b`, then we will + // find that `tmp`, which is the result of getting the + // parent when treating the argument as a path, contains + // at least three consecutive Xs. This means that there + // was a path separator in the suffix, which is not + // allowed. + if tmp.display().to_string().contains("XXX") { + return Err(MkTempError::SuffixContainsDirSeparator(format!( + "{}{}", + MAIN_SEPARATOR, template + )) + .into()); + } (template, tmp) } } else { diff --git a/tests/by-util/test_mktemp.rs b/tests/by-util/test_mktemp.rs index 9fce0e591..962ab565b 100644 --- a/tests/by-util/test_mktemp.rs +++ b/tests/by-util/test_mktemp.rs @@ -496,3 +496,18 @@ fn test_template_path_separator() { "a/bXXX".quote() )); } + +/// Test that a suffix with a path separator is invalid. +#[test] +fn test_suffix_path_separator() { + #[cfg(not(windows))] + new_ucmd!() + .arg("aXXX/b") + .fails() + .stderr_only("mktemp: invalid suffix '/b', contains directory separator\n"); + #[cfg(windows)] + new_ucmd!() + .arg(r"aXXX\b") + .fails() + .stderr_only("mktemp: invalid suffix '\\b', contains directory separator\n"); +} From 7a599c26ff63fad3be500a1a76f44668d5f84d21 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Fri, 20 May 2022 09:27:34 +0200 Subject: [PATCH 09/11] dependabot: remove trailing space --- .github/dependabot.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/dependabot.yml b/.github/dependabot.yml index cfcddbb14..38f046ead 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -7,6 +7,6 @@ updates: open-pull-requests-limit: 5 - package-ecosystem: "github-actions" directory: "/" - schedule: + schedule: interval: daily open-pull-requests-limit: 5 From af5ef5585f7580ffa1991b6ff25926635b3f133b Mon Sep 17 00:00:00 2001 From: Daniel Hofstetter Date: Fri, 20 May 2022 15:10:16 +0200 Subject: [PATCH 10/11] df/uniq: suppress lint errors --- src/uu/df/src/df.rs | 1 + src/uu/uniq/src/uniq.rs | 3 +++ 2 files changed, 4 insertions(+) diff --git a/src/uu/df/src/df.rs b/src/uu/df/src/df.rs index 793138f7d..7c95e6938 100644 --- a/src/uu/df/src/df.rs +++ b/src/uu/df/src/df.rs @@ -148,6 +148,7 @@ impl fmt::Display for OptionsError { "option --output: field {} used more than once", s.quote() ), + #[allow(clippy::print_in_format_impl)] Self::FilesystemTypeBothSelectedAndExcluded(types) => { for t in types { eprintln!( diff --git a/src/uu/uniq/src/uniq.rs b/src/uu/uniq/src/uniq.rs index e71c21303..d947166fe 100644 --- a/src/uu/uniq/src/uniq.rs +++ b/src/uu/uniq/src/uniq.rs @@ -5,6 +5,9 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. +// TODO remove this when https://github.com/rust-lang/rust-clippy/issues/6902 is fixed +#![allow(clippy::use_self)] + use clap::{crate_version, Arg, ArgMatches, Command}; use std::fs::File; use std::io::{self, stdin, stdout, BufRead, BufReader, BufWriter, Read, Write}; From 3ea6720d46f836fd82081da25bac5c46548c2af7 Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru Date: Sat, 21 May 2022 09:22:24 +0200 Subject: [PATCH 11/11] Update extension ID for rust-analyzer vscode extension recommendation --- .vscode/extensions.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.vscode/extensions.json b/.vscode/extensions.json index bd9dcf485..b9c161a4c 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -6,7 +6,7 @@ // "streetsidesoftware.code-spell-checker" ~ `cspell` spell-checker support { "recommendations": [ - "matklad.rust-analyzer", + "rust-lang.rust-analyzer", "streetsidesoftware.code-spell-checker", "foxundermoon.shell-format" ]