From c5d7cbda32f443964559c3e66f1c3360b561af4c Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 16:59:06 +0200 Subject: [PATCH 01/11] timeout: handle arguments for the command to run To prevent clap from parsing flags for the command to run as flags for timeout, remove the "args" positional argument, but allow to pass flags via the "command" positional arg. --- src/uu/timeout/src/timeout.rs | 28 ++++++++++------------------ tests/by-util/test_timeout.rs | 8 ++++++++ 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index afe560ee5..ea9a0dc65 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -37,7 +37,6 @@ pub mod options { // Positional args. pub static DURATION: &str = "duration"; pub static COMMAND: &str = "command"; - pub static ARGS: &str = "args"; } struct Config { @@ -47,8 +46,7 @@ struct Config { duration: Duration, preserve_status: bool, - command: String, - command_args: Vec, + command: Vec, } impl Config { @@ -77,12 +75,11 @@ impl Config { let preserve_status: bool = options.is_present(options::PRESERVE_STATUS); let foreground = options.is_present(options::FOREGROUND); - let command: String = options.value_of(options::COMMAND).unwrap().to_string(); - - let command_args: Vec = match options.values_of(options::ARGS) { - Some(values) => values.map(|x| x.to_owned()).collect(), - None => vec![], - }; + let command = options + .values_of(options::COMMAND) + .unwrap() + .map(String::from) + .collect::>(); Config { foreground, @@ -91,7 +88,6 @@ impl Config { duration, preserve_status, command, - command_args, } } } @@ -137,9 +133,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { Arg::with_name(options::COMMAND) .index(2) .required(true) - ) - .arg( - Arg::with_name(options::ARGS).multiple(true) + .multiple(true) ) .setting(AppSettings::TrailingVarArg); @@ -148,7 +142,6 @@ pub fn uumain(args: impl uucore::Args) -> i32 { let config = Config::from(matches); timeout( &config.command, - &config.command_args, config.duration, config.signal, config.kill_after, @@ -160,8 +153,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { /// TODO: Improve exit codes, and make them consistent with the GNU Coreutils exit codes. fn timeout( - cmdname: &str, - args: &[String], + cmd: &[String], duration: Duration, signal: usize, kill_after: Duration, @@ -171,8 +163,8 @@ fn timeout( if !foreground { unsafe { libc::setpgid(0, 0) }; } - let mut process = match Command::new(cmdname) - .args(args) + let mut process = match Command::new(&cmd[0]) + .args(&cmd[1..]) .stdin(Stdio::inherit()) .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index 28273e00f..2346a17aa 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -9,3 +9,11 @@ fn test_subcommand_return_code() { new_ucmd!().arg("1").arg("false").run().status_code(1); } + +#[test] +fn test_command_with_args() { + new_ucmd!() + .args(&["1700", "echo", "-n", "abcd"]) + .succeeds() + .stdout_only("abcd"); +} From ed646090c209fe4376d20439ecf6279b6ee26131 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 17:00:12 +0200 Subject: [PATCH 02/11] timeout: fix usage string --- src/uu/timeout/src/timeout.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index ea9a0dc65..3a35c351f 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -23,7 +23,7 @@ use uucore::InvalidEncodingHandling; static ABOUT: &str = "Start COMMAND, and kill it if still running after DURATION."; fn get_usage() -> String { - format!("{0} [OPTION]... [FILE]...", executable!()) + format!("{0} [OPTION] DURATION COMMAND...", executable!()) } const ERR_EXIT_STATUS: i32 = 125; From ceb5a2998c18215bad1684d337da3220cb8201d0 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 18:28:37 +0200 Subject: [PATCH 03/11] core: add EXIT signal EXIT is supported by GNU (see https://github.com/coreutils/gnulib/blob/993ca832d232c33da1d2bb07e91acd6d301ebea0/lib/sig2str.c#L258), so we have to support it too to pass GNU tests. --- src/uucore/src/lib/features/signals.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/uucore/src/lib/features/signals.rs b/src/uucore/src/lib/features/signals.rs index d22fa1791..2e6069cb7 100644 --- a/src/uucore/src/lib/features/signals.rs +++ b/src/uucore/src/lib/features/signals.rs @@ -29,7 +29,11 @@ Linux Programmer's Manual */ #[cfg(target_os = "linux")] -pub static ALL_SIGNALS: [Signal<'static>; 31] = [ +pub static ALL_SIGNALS: [Signal<'static>; 32] = [ + Signal { + name: "EXIT", + value: 0, + }, Signal { name: "HUP", value: 1, @@ -198,7 +202,11 @@ No Name Default Action Description */ #[cfg(any(target_vendor = "apple", target_os = "freebsd"))] -pub static ALL_SIGNALS: [Signal<'static>; 31] = [ +pub static ALL_SIGNALS: [Signal<'static>; 32] = [ + Signal { + name: "EXIT", + value: 0, + }, Signal { name: "HUP", value: 1, From b0b937dc3e2a81c95b0d621f726fd9b39673dfa6 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 18:29:06 +0200 Subject: [PATCH 04/11] core: add signal name lookup by value --- src/uucore/src/lib/features/signals.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/uucore/src/lib/features/signals.rs b/src/uucore/src/lib/features/signals.rs index 2e6069cb7..3c52a9158 100644 --- a/src/uucore/src/lib/features/signals.rs +++ b/src/uucore/src/lib/features/signals.rs @@ -349,6 +349,13 @@ pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option { .map(|s| s.value) } +pub fn signal_name_by_value(signal_value: usize) -> Option<&'static str> { + ALL_SIGNALS + .iter() + .find(|signal| signal.value == signal_value) + .map(|signal| signal.name) +} + #[inline(always)] pub fn is_signal(num: usize) -> bool { // Named signals start at 1 @@ -358,7 +365,7 @@ pub fn is_signal(num: usize) -> bool { #[test] fn signals_all_contiguous() { for (i, signal) in ALL_SIGNALS.iter().enumerate() { - assert_eq!(signal.value, i + 1); + assert_eq!(signal.value, i); } } @@ -396,3 +403,10 @@ fn signal_by_long_name() { ); } } + +#[test] +fn name() { + for signal in &ALL_SIGNALS { + assert_eq!(signal_name_by_value(signal.value), Some(signal.name)); + } +} From 8e0ed2d20e03b56a7ade4f41ff3e1ab71bcb5e41 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 18:34:05 +0200 Subject: [PATCH 05/11] timeout: support --verbose --- src/uu/timeout/src/timeout.rs | 24 +++++++++++++++++++++++- tests/by-util/test_timeout.rs | 14 ++++++++++++++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index 3a35c351f..3c82c4be2 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -17,7 +17,7 @@ use std::io::ErrorKind; use std::process::{Command, Stdio}; use std::time::Duration; use uucore::process::ChildExt; -use uucore::signals::signal_by_name_or_value; +use uucore::signals::{signal_by_name_or_value, signal_name_by_value}; use uucore::InvalidEncodingHandling; static ABOUT: &str = "Start COMMAND, and kill it if still running after DURATION."; @@ -33,6 +33,7 @@ pub mod options { pub static KILL_AFTER: &str = "kill-after"; pub static SIGNAL: &str = "signal"; pub static PRESERVE_STATUS: &str = "preserve-status"; + pub static VERBOSE: &str = "verbose"; // Positional args. pub static DURATION: &str = "duration"; @@ -45,6 +46,7 @@ struct Config { signal: usize, duration: Duration, preserve_status: bool, + verbose: bool, command: Vec, } @@ -74,6 +76,7 @@ impl Config { let preserve_status: bool = options.is_present(options::PRESERVE_STATUS); let foreground = options.is_present(options::FOREGROUND); + let verbose = options.is_present(options::VERBOSE); let command = options .values_of(options::COMMAND) @@ -88,6 +91,7 @@ impl Config { duration, preserve_status, command, + verbose, } } } @@ -124,6 +128,12 @@ pub fn uumain(args: impl uucore::Args) -> i32 { .help("specify the signal to be sent on timeout; SIGNAL may be a name like 'HUP' or a number; see 'kill -l' for a list of signals") .takes_value(true) ) + .arg( + Arg::with_name(options::VERBOSE) + .short("v") + .long(options::VERBOSE) + .help("diagnose to stderr any signal sent upon timeout") + ) .arg( Arg::with_name(options::DURATION) .index(1) @@ -147,6 +157,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 { config.kill_after, config.foreground, config.preserve_status, + config.verbose, ) } @@ -159,6 +170,7 @@ fn timeout( kill_after: Duration, foreground: bool, preserve_status: bool, + verbose: bool, ) -> i32 { if !foreground { unsafe { libc::setpgid(0, 0) }; @@ -185,6 +197,13 @@ fn timeout( match process.wait_or_timeout(duration) { Ok(Some(status)) => status.code().unwrap_or_else(|| status.signal().unwrap()), Ok(None) => { + if verbose { + show_error!( + "sending signal {} to command '{}'", + signal_name_by_value(signal).unwrap(), + cmd[0] + ); + } return_if_err!(ERR_EXIT_STATUS, process.send_signal(signal)); match process.wait_or_timeout(kill_after) { Ok(Some(status)) => { @@ -199,6 +218,9 @@ fn timeout( // XXX: this may not be right return 124; } + if verbose { + show_error!("sending signal KILL to command '{}'", cmd[0]); + } return_if_err!( ERR_EXIT_STATUS, process diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index 2346a17aa..4d2451c7e 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -17,3 +17,17 @@ fn test_command_with_args() { .succeeds() .stdout_only("abcd"); } + +#[test] +fn test_verbose() { + for &verbose_flag in &["-v", "--verbose"] { + new_ucmd!() + .args(&[verbose_flag, ".1", "sleep", "10"]) + .fails() + .stderr_only("timeout: sending signal TERM to command 'sleep'"); + new_ucmd!() + .args(&[verbose_flag, "-s0", "-k.1", ".1", "sleep", "10"]) + .fails() + .stderr_only("timeout: sending signal EXIT to command 'sleep'\ntimeout: sending signal KILL to command 'sleep'"); + } +} From 0f9bc8e9746809e1fe9b59e656e62f395e347610 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 18:51:41 +0200 Subject: [PATCH 06/11] timeout: disable timeout if it is set to zero --- src/uu/timeout/src/timeout.rs | 4 ---- src/uucore/src/lib/features/process.rs | 6 ++++++ tests/by-util/test_timeout.rs | 14 ++++++++++++++ 3 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index 3c82c4be2..b671d9d3e 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -214,10 +214,6 @@ fn timeout( } } Ok(None) => { - if kill_after == Duration::new(0, 0) { - // XXX: this may not be right - return 124; - } if verbose { show_error!("sending signal KILL to command '{}'", cmd[0]); } diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index 975123cf7..cda41bb4f 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -93,6 +93,7 @@ pub trait ChildExt { fn send_signal(&mut self, signal: usize) -> io::Result<()>; /// Wait for a process to finish or return after the specified duration. + /// A `timeout` of zero disables the timeout. fn wait_or_timeout(&mut self, timeout: Duration) -> io::Result>; } @@ -106,6 +107,11 @@ impl ChildExt for Child { } fn wait_or_timeout(&mut self, timeout: Duration) -> io::Result> { + if timeout == Duration::from_micros(0) { + return self + .wait() + .map(|status| Some(ExitStatus::from_std_status(status))); + } // .try_wait() doesn't drop stdin, so we do it manually drop(self.stdin.take()); diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index 4d2451c7e..9be29065a 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -31,3 +31,17 @@ fn test_verbose() { .stderr_only("timeout: sending signal EXIT to command 'sleep'\ntimeout: sending signal KILL to command 'sleep'"); } } + +#[test] +fn test_zero_timeout() { + new_ucmd!() + .args(&["-v", "0", "sleep", ".1"]) + .succeeds() + .no_stderr() + .no_stdout(); + new_ucmd!() + .args(&["-v", "0", "-s0", "-k0", "sleep", ".1"]) + .succeeds() + .no_stderr() + .no_stdout(); +} From b4efd5a749592758bf0d813f6bd2668b813a1e66 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Thu, 10 Jun 2021 20:03:33 +0200 Subject: [PATCH 07/11] timeout: disable pre-existing SIGCHLD handlers Needed to make a GNU test pass --- Cargo.lock | 3 +++ src/uu/timeout/Cargo.toml | 1 + src/uu/timeout/src/timeout.rs | 14 +++++++++++++- 3 files changed, 17 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index c2c3a6242..6e9c22049 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,5 +1,7 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. +version = 3 + [[package]] name = "Inflector" version = "0.11.4" @@ -2606,6 +2608,7 @@ version = "0.0.6" dependencies = [ "clap", "libc", + "nix 0.20.0", "uucore", "uucore_procs", ] diff --git a/src/uu/timeout/Cargo.toml b/src/uu/timeout/Cargo.toml index d16559858..a09342c0a 100644 --- a/src/uu/timeout/Cargo.toml +++ b/src/uu/timeout/Cargo.toml @@ -17,6 +17,7 @@ path = "src/timeout.rs" [dependencies] clap = "2.33" libc = "0.2.42" +nix = "0.20.0" uucore = { version=">=0.0.8", package="uucore", path="../../uucore", features=["parse_time", "process", "signals"] } uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" } diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index b671d9d3e..eabf0192a 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -5,7 +5,7 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -// spell-checker:ignore (ToDO) tstr sigstr cmdname setpgid +// spell-checker:ignore (ToDO) tstr sigstr cmdname setpgid sigchld #[macro_use] extern crate uucore; @@ -161,6 +161,17 @@ pub fn uumain(args: impl uucore::Args) -> i32 { ) } +/// Remove pre-existing SIGCHLD handlers that would make waiting for the child's exit code fail. +fn unblock_sigchld() { + unsafe { + nix::sys::signal::signal( + nix::sys::signal::Signal::SIGCHLD, + nix::sys::signal::SigHandler::SigDfl, + ) + .unwrap(); + } +} + /// TODO: Improve exit codes, and make them consistent with the GNU Coreutils exit codes. fn timeout( @@ -194,6 +205,7 @@ fn timeout( } } }; + unblock_sigchld(); match process.wait_or_timeout(duration) { Ok(Some(status)) => status.code().unwrap_or_else(|| status.signal().unwrap()), Ok(None) => { From f90975115538ba4d2043113f4cecf39bd17af94c Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Fri, 11 Jun 2021 20:44:25 +0200 Subject: [PATCH 08/11] timeout: don't kill the process if -k is not set `timeout` used to set the timeout to 0 when -k was not set. This collided with the behavior of 0 timeouts, which disable the timeout. When -k is not set the process should not be killed. --- src/uu/timeout/src/timeout.rs | 52 +++++++++++++++++++---------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index eabf0192a..bc92157ca 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -42,7 +42,7 @@ pub mod options { struct Config { foreground: bool, - kill_after: Duration, + kill_after: Option, signal: usize, duration: Duration, preserve_status: bool, @@ -66,10 +66,9 @@ impl Config { _ => uucore::signals::signal_by_name_or_value("TERM").unwrap(), }; - let kill_after: Duration = match options.value_of(options::KILL_AFTER) { - Some(time) => uucore::parse_time::from_str(time).unwrap(), - None => Duration::new(0, 0), - }; + let kill_after = options + .value_of(options::KILL_AFTER) + .map(|time| uucore::parse_time::from_str(time).unwrap()); let duration: Duration = uucore::parse_time::from_str(options.value_of(options::DURATION).unwrap()).unwrap(); @@ -178,7 +177,7 @@ fn timeout( cmd: &[String], duration: Duration, signal: usize, - kill_after: Duration, + kill_after: Option, foreground: bool, preserve_status: bool, verbose: bool, @@ -217,27 +216,32 @@ fn timeout( ); } return_if_err!(ERR_EXIT_STATUS, process.send_signal(signal)); - match process.wait_or_timeout(kill_after) { - Ok(Some(status)) => { - if preserve_status { - status.code().unwrap_or_else(|| status.signal().unwrap()) - } else { - 124 + if let Some(kill_after) = kill_after { + match process.wait_or_timeout(kill_after) { + Ok(Some(status)) => { + if preserve_status { + status.code().unwrap_or_else(|| status.signal().unwrap()) + } else { + 124 + } } - } - Ok(None) => { - if verbose { - show_error!("sending signal KILL to command '{}'", cmd[0]); + Ok(None) => { + if verbose { + show_error!("sending signal KILL to command '{}'", cmd[0]); + } + return_if_err!( + ERR_EXIT_STATUS, + process.send_signal( + uucore::signals::signal_by_name_or_value("KILL").unwrap() + ) + ); + return_if_err!(ERR_EXIT_STATUS, process.wait()); + 137 } - return_if_err!( - ERR_EXIT_STATUS, - process - .send_signal(uucore::signals::signal_by_name_or_value("KILL").unwrap()) - ); - return_if_err!(ERR_EXIT_STATUS, process.wait()); - 137 + Err(_) => 124, } - Err(_) => 124, + } else { + 124 } } Err(_) => { From a390383d2d71f51a4512abb641e61655ac280484 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 12 Jun 2021 13:01:42 +0200 Subject: [PATCH 09/11] core: represent signal values by their index --- src/uucore/src/lib/features/signals.rs | 326 ++----------------------- 1 file changed, 23 insertions(+), 303 deletions(-) diff --git a/src/uucore/src/lib/features/signals.rs b/src/uucore/src/lib/features/signals.rs index 3c52a9158..e6d2e7763 100644 --- a/src/uucore/src/lib/features/signals.rs +++ b/src/uucore/src/lib/features/signals.rs @@ -10,11 +10,6 @@ pub static DEFAULT_SIGNAL: usize = 15; -pub struct Signal<'a> { - pub name: &'a str, - pub value: usize, -} - /* Linux Programmer's Manual @@ -29,135 +24,10 @@ Linux Programmer's Manual */ #[cfg(target_os = "linux")] -pub static ALL_SIGNALS: [Signal<'static>; 32] = [ - Signal { - name: "EXIT", - value: 0, - }, - Signal { - name: "HUP", - value: 1, - }, - Signal { - name: "INT", - value: 2, - }, - Signal { - name: "QUIT", - value: 3, - }, - Signal { - name: "ILL", - value: 4, - }, - Signal { - name: "TRAP", - value: 5, - }, - Signal { - name: "ABRT", - value: 6, - }, - Signal { - name: "BUS", - value: 7, - }, - Signal { - name: "FPE", - value: 8, - }, - Signal { - name: "KILL", - value: 9, - }, - Signal { - name: "USR1", - value: 10, - }, - Signal { - name: "SEGV", - value: 11, - }, - Signal { - name: "USR2", - value: 12, - }, - Signal { - name: "PIPE", - value: 13, - }, - Signal { - name: "ALRM", - value: 14, - }, - Signal { - name: "TERM", - value: 15, - }, - Signal { - name: "STKFLT", - value: 16, - }, - Signal { - name: "CHLD", - value: 17, - }, - Signal { - name: "CONT", - value: 18, - }, - Signal { - name: "STOP", - value: 19, - }, - Signal { - name: "TSTP", - value: 20, - }, - Signal { - name: "TTIN", - value: 21, - }, - Signal { - name: "TTOU", - value: 22, - }, - Signal { - name: "URG", - value: 23, - }, - Signal { - name: "XCPU", - value: 24, - }, - Signal { - name: "XFSZ", - value: 25, - }, - Signal { - name: "VTALRM", - value: 26, - }, - Signal { - name: "PROF", - value: 27, - }, - Signal { - name: "WINCH", - value: 28, - }, - Signal { - name: "POLL", - value: 29, - }, - Signal { - name: "PWR", - value: 30, - }, - Signal { - name: "SYS", - value: 31, - }, +pub static ALL_SIGNALS: [&str; 32] = [ + "EXIT", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "BUS", "FPE", "KILL", "USR1", "SEGV", + "USR2", "PIPE", "ALRM", "TERM", "STKFLT", "CHLD", "CONT", "STOP", "TSTP", "TTIN", "TTOU", + "URG", "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "POLL", "PWR", "SYS", ]; /* @@ -202,135 +72,10 @@ No Name Default Action Description */ #[cfg(any(target_vendor = "apple", target_os = "freebsd"))] -pub static ALL_SIGNALS: [Signal<'static>; 32] = [ - Signal { - name: "EXIT", - value: 0, - }, - Signal { - name: "HUP", - value: 1, - }, - Signal { - name: "INT", - value: 2, - }, - Signal { - name: "QUIT", - value: 3, - }, - Signal { - name: "ILL", - value: 4, - }, - Signal { - name: "TRAP", - value: 5, - }, - Signal { - name: "ABRT", - value: 6, - }, - Signal { - name: "EMT", - value: 7, - }, - Signal { - name: "FPE", - value: 8, - }, - Signal { - name: "KILL", - value: 9, - }, - Signal { - name: "BUS", - value: 10, - }, - Signal { - name: "SEGV", - value: 11, - }, - Signal { - name: "SYS", - value: 12, - }, - Signal { - name: "PIPE", - value: 13, - }, - Signal { - name: "ALRM", - value: 14, - }, - Signal { - name: "TERM", - value: 15, - }, - Signal { - name: "URG", - value: 16, - }, - Signal { - name: "STOP", - value: 17, - }, - Signal { - name: "TSTP", - value: 18, - }, - Signal { - name: "CONT", - value: 19, - }, - Signal { - name: "CHLD", - value: 20, - }, - Signal { - name: "TTIN", - value: 21, - }, - Signal { - name: "TTOU", - value: 22, - }, - Signal { - name: "IO", - value: 23, - }, - Signal { - name: "XCPU", - value: 24, - }, - Signal { - name: "XFSZ", - value: 25, - }, - Signal { - name: "VTALRM", - value: 26, - }, - Signal { - name: "PROF", - value: 27, - }, - Signal { - name: "WINCH", - value: 28, - }, - Signal { - name: "INFO", - value: 29, - }, - Signal { - name: "USR1", - value: 30, - }, - Signal { - name: "USR2", - value: 31, - }, +pub static ALL_SIGNALS: [&str; 32] = [ + "EXIT", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "EMT", "FPE", "KILL", "BUS", "SEGV", + "SYS", "PIPE", "ALRM", "TERM", "URG", "STOP", "TSTP", "CONT", "CHLD", "TTIN", "TTOU", "IO", + "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "INFO", "USR1", "USR2", ]; pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option { @@ -343,70 +88,45 @@ pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option { } let signal_name = signal_name_or_value.trim_start_matches("SIG"); - ALL_SIGNALS - .iter() - .find(|s| s.name == signal_name) - .map(|s| s.value) + ALL_SIGNALS.iter().position(|&s| s == signal_name) +} + +pub fn is_signal(num: usize) -> bool { + num < ALL_SIGNALS.len() } pub fn signal_name_by_value(signal_value: usize) -> Option<&'static str> { - ALL_SIGNALS - .iter() - .find(|signal| signal.value == signal_value) - .map(|signal| signal.name) -} - -#[inline(always)] -pub fn is_signal(num: usize) -> bool { - // Named signals start at 1 - num <= ALL_SIGNALS.len() -} - -#[test] -fn signals_all_contiguous() { - for (i, signal) in ALL_SIGNALS.iter().enumerate() { - assert_eq!(signal.value, i); - } -} - -#[test] -fn signals_all_are_signal() { - for signal in &ALL_SIGNALS { - assert!(is_signal(signal.value)); - } + ALL_SIGNALS.get(signal_value).copied() } #[test] fn signal_by_value() { assert_eq!(signal_by_name_or_value("0"), Some(0)); - for signal in &ALL_SIGNALS { - assert_eq!( - signal_by_name_or_value(&signal.value.to_string()), - Some(signal.value) - ); + for (value, _signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_by_name_or_value(&value.to_string()), Some(value)); } } #[test] fn signal_by_short_name() { - for signal in &ALL_SIGNALS { - assert_eq!(signal_by_name_or_value(signal.name), Some(signal.value)); + for (value, signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_by_name_or_value(signal), Some(value)); } } #[test] fn signal_by_long_name() { - for signal in &ALL_SIGNALS { + for (value, signal) in ALL_SIGNALS.iter().enumerate() { assert_eq!( - signal_by_name_or_value(&format!("SIG{}", signal.name)), - Some(signal.value) + signal_by_name_or_value(&format!("SIG{}", signal)), + Some(value) ); } } #[test] fn name() { - for signal in &ALL_SIGNALS { - assert_eq!(signal_name_by_value(signal.value), Some(signal.name)); + for (value, signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_name_by_value(value), Some(*signal)); } } From a57313f01b29f2b363eea3dcace5d2ceeaa02754 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Sat, 12 Jun 2021 13:01:42 +0200 Subject: [PATCH 10/11] core: represent signal values by their index --- src/uu/kill/src/kill.rs | 22 +- src/uucore/src/lib/features/signals.rs | 326 ++----------------------- 2 files changed, 33 insertions(+), 315 deletions(-) diff --git a/src/uu/kill/src/kill.rs b/src/uu/kill/src/kill.rs index a49acaa05..7965c40a9 100644 --- a/src/uu/kill/src/kill.rs +++ b/src/uu/kill/src/kill.rs @@ -132,13 +132,13 @@ fn table() { let mut name_width = 0; /* Compute the maximum width of a signal name. */ for s in &ALL_SIGNALS { - if s.name.len() > name_width { - name_width = s.name.len() + if s.len() > name_width { + name_width = s.len() } } for (idx, signal) in ALL_SIGNALS.iter().enumerate() { - print!("{0: >#2} {1: <#8}", idx + 1, signal.name); + print!("{0: >#2} {1: <#8}", idx + 1, signal); //TODO: obtain max signal width here if (idx + 1) % 7 == 0 { @@ -148,14 +148,12 @@ fn table() { } fn print_signal(signal_name_or_value: &str) { - for signal in &ALL_SIGNALS { - if signal.name == signal_name_or_value - || (format!("SIG{}", signal.name)) == signal_name_or_value - { - println!("{}", signal.value); + for (value, &signal) in ALL_SIGNALS.iter().enumerate() { + if signal == signal_name_or_value || (format!("SIG{}", signal)) == signal_name_or_value { + println!("{}", value); exit!(EXIT_OK as i32) - } else if signal_name_or_value == signal.value.to_string() { - println!("{}", signal.name); + } else if signal_name_or_value == value.to_string() { + println!("{}", signal); exit!(EXIT_OK as i32) } } @@ -165,8 +163,8 @@ fn print_signal(signal_name_or_value: &str) { fn print_signals() { let mut pos = 0; for (idx, signal) in ALL_SIGNALS.iter().enumerate() { - pos += signal.name.len(); - print!("{}", signal.name); + pos += signal.len(); + print!("{}", signal); if idx > 0 && pos > 73 { println!(); pos = 0; diff --git a/src/uucore/src/lib/features/signals.rs b/src/uucore/src/lib/features/signals.rs index 3c52a9158..e6d2e7763 100644 --- a/src/uucore/src/lib/features/signals.rs +++ b/src/uucore/src/lib/features/signals.rs @@ -10,11 +10,6 @@ pub static DEFAULT_SIGNAL: usize = 15; -pub struct Signal<'a> { - pub name: &'a str, - pub value: usize, -} - /* Linux Programmer's Manual @@ -29,135 +24,10 @@ Linux Programmer's Manual */ #[cfg(target_os = "linux")] -pub static ALL_SIGNALS: [Signal<'static>; 32] = [ - Signal { - name: "EXIT", - value: 0, - }, - Signal { - name: "HUP", - value: 1, - }, - Signal { - name: "INT", - value: 2, - }, - Signal { - name: "QUIT", - value: 3, - }, - Signal { - name: "ILL", - value: 4, - }, - Signal { - name: "TRAP", - value: 5, - }, - Signal { - name: "ABRT", - value: 6, - }, - Signal { - name: "BUS", - value: 7, - }, - Signal { - name: "FPE", - value: 8, - }, - Signal { - name: "KILL", - value: 9, - }, - Signal { - name: "USR1", - value: 10, - }, - Signal { - name: "SEGV", - value: 11, - }, - Signal { - name: "USR2", - value: 12, - }, - Signal { - name: "PIPE", - value: 13, - }, - Signal { - name: "ALRM", - value: 14, - }, - Signal { - name: "TERM", - value: 15, - }, - Signal { - name: "STKFLT", - value: 16, - }, - Signal { - name: "CHLD", - value: 17, - }, - Signal { - name: "CONT", - value: 18, - }, - Signal { - name: "STOP", - value: 19, - }, - Signal { - name: "TSTP", - value: 20, - }, - Signal { - name: "TTIN", - value: 21, - }, - Signal { - name: "TTOU", - value: 22, - }, - Signal { - name: "URG", - value: 23, - }, - Signal { - name: "XCPU", - value: 24, - }, - Signal { - name: "XFSZ", - value: 25, - }, - Signal { - name: "VTALRM", - value: 26, - }, - Signal { - name: "PROF", - value: 27, - }, - Signal { - name: "WINCH", - value: 28, - }, - Signal { - name: "POLL", - value: 29, - }, - Signal { - name: "PWR", - value: 30, - }, - Signal { - name: "SYS", - value: 31, - }, +pub static ALL_SIGNALS: [&str; 32] = [ + "EXIT", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "BUS", "FPE", "KILL", "USR1", "SEGV", + "USR2", "PIPE", "ALRM", "TERM", "STKFLT", "CHLD", "CONT", "STOP", "TSTP", "TTIN", "TTOU", + "URG", "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "POLL", "PWR", "SYS", ]; /* @@ -202,135 +72,10 @@ No Name Default Action Description */ #[cfg(any(target_vendor = "apple", target_os = "freebsd"))] -pub static ALL_SIGNALS: [Signal<'static>; 32] = [ - Signal { - name: "EXIT", - value: 0, - }, - Signal { - name: "HUP", - value: 1, - }, - Signal { - name: "INT", - value: 2, - }, - Signal { - name: "QUIT", - value: 3, - }, - Signal { - name: "ILL", - value: 4, - }, - Signal { - name: "TRAP", - value: 5, - }, - Signal { - name: "ABRT", - value: 6, - }, - Signal { - name: "EMT", - value: 7, - }, - Signal { - name: "FPE", - value: 8, - }, - Signal { - name: "KILL", - value: 9, - }, - Signal { - name: "BUS", - value: 10, - }, - Signal { - name: "SEGV", - value: 11, - }, - Signal { - name: "SYS", - value: 12, - }, - Signal { - name: "PIPE", - value: 13, - }, - Signal { - name: "ALRM", - value: 14, - }, - Signal { - name: "TERM", - value: 15, - }, - Signal { - name: "URG", - value: 16, - }, - Signal { - name: "STOP", - value: 17, - }, - Signal { - name: "TSTP", - value: 18, - }, - Signal { - name: "CONT", - value: 19, - }, - Signal { - name: "CHLD", - value: 20, - }, - Signal { - name: "TTIN", - value: 21, - }, - Signal { - name: "TTOU", - value: 22, - }, - Signal { - name: "IO", - value: 23, - }, - Signal { - name: "XCPU", - value: 24, - }, - Signal { - name: "XFSZ", - value: 25, - }, - Signal { - name: "VTALRM", - value: 26, - }, - Signal { - name: "PROF", - value: 27, - }, - Signal { - name: "WINCH", - value: 28, - }, - Signal { - name: "INFO", - value: 29, - }, - Signal { - name: "USR1", - value: 30, - }, - Signal { - name: "USR2", - value: 31, - }, +pub static ALL_SIGNALS: [&str; 32] = [ + "EXIT", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "EMT", "FPE", "KILL", "BUS", "SEGV", + "SYS", "PIPE", "ALRM", "TERM", "URG", "STOP", "TSTP", "CONT", "CHLD", "TTIN", "TTOU", "IO", + "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "INFO", "USR1", "USR2", ]; pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option { @@ -343,70 +88,45 @@ pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option { } let signal_name = signal_name_or_value.trim_start_matches("SIG"); - ALL_SIGNALS - .iter() - .find(|s| s.name == signal_name) - .map(|s| s.value) + ALL_SIGNALS.iter().position(|&s| s == signal_name) +} + +pub fn is_signal(num: usize) -> bool { + num < ALL_SIGNALS.len() } pub fn signal_name_by_value(signal_value: usize) -> Option<&'static str> { - ALL_SIGNALS - .iter() - .find(|signal| signal.value == signal_value) - .map(|signal| signal.name) -} - -#[inline(always)] -pub fn is_signal(num: usize) -> bool { - // Named signals start at 1 - num <= ALL_SIGNALS.len() -} - -#[test] -fn signals_all_contiguous() { - for (i, signal) in ALL_SIGNALS.iter().enumerate() { - assert_eq!(signal.value, i); - } -} - -#[test] -fn signals_all_are_signal() { - for signal in &ALL_SIGNALS { - assert!(is_signal(signal.value)); - } + ALL_SIGNALS.get(signal_value).copied() } #[test] fn signal_by_value() { assert_eq!(signal_by_name_or_value("0"), Some(0)); - for signal in &ALL_SIGNALS { - assert_eq!( - signal_by_name_or_value(&signal.value.to_string()), - Some(signal.value) - ); + for (value, _signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_by_name_or_value(&value.to_string()), Some(value)); } } #[test] fn signal_by_short_name() { - for signal in &ALL_SIGNALS { - assert_eq!(signal_by_name_or_value(signal.name), Some(signal.value)); + for (value, signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_by_name_or_value(signal), Some(value)); } } #[test] fn signal_by_long_name() { - for signal in &ALL_SIGNALS { + for (value, signal) in ALL_SIGNALS.iter().enumerate() { assert_eq!( - signal_by_name_or_value(&format!("SIG{}", signal.name)), - Some(signal.value) + signal_by_name_or_value(&format!("SIG{}", signal)), + Some(value) ); } } #[test] fn name() { - for signal in &ALL_SIGNALS { - assert_eq!(signal_name_by_value(signal.value), Some(signal.name)); + for (value, signal) in ALL_SIGNALS.iter().enumerate() { + assert_eq!(signal_name_by_value(value), Some(*signal)); } } From 7acb9373a62270c5baee091019da88f43ebc16b8 Mon Sep 17 00:00:00 2001 From: Michael Debertol Date: Mon, 14 Jun 2021 11:10:41 +0200 Subject: [PATCH 11/11] kill: fix signal table printing --- src/uu/kill/src/kill.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/uu/kill/src/kill.rs b/src/uu/kill/src/kill.rs index 7965c40a9..c48864564 100644 --- a/src/uu/kill/src/kill.rs +++ b/src/uu/kill/src/kill.rs @@ -129,22 +129,15 @@ fn handle_obsolete(mut args: Vec) -> (Vec, Option) { } fn table() { - let mut name_width = 0; - /* Compute the maximum width of a signal name. */ - for s in &ALL_SIGNALS { - if s.len() > name_width { - name_width = s.len() - } - } + let name_width = ALL_SIGNALS.iter().map(|n| n.len()).max().unwrap(); for (idx, signal) in ALL_SIGNALS.iter().enumerate() { - print!("{0: >#2} {1: <#8}", idx + 1, signal); - //TODO: obtain max signal width here - + print!("{0: >#2} {1: <#2$}", idx, signal, name_width + 2); if (idx + 1) % 7 == 0 { println!(); } } + println!() } fn print_signal(signal_name_or_value: &str) {