Merge pull request #2395 from miDeb/timeout/cmd-args

timeout: handle arguments for the command to run

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"
@@ -2597,6 +2599,7 @@ version = "0.0.6"
 dependencies = [
  "clap",
  "libc",
+ "nix 0.20.0",
  "uucore",
  "uucore_procs",
 ]

src/uu/kill/src/kill.rs

@@ -129,33 +129,24 @@ fn handle_obsolete(mut args: Vec<String>) -> (Vec<String>, Option<String>) {
 }
 
 fn table() {
-    let mut name_width = 0;
+    let name_width = ALL_SIGNALS.iter().map(|n| n.len()).max().unwrap();
-    /* Compute the maximum width of a signal name. */
-    for s in &ALL_SIGNALS {
-        if s.name.len() > name_width {
-            name_width = s.name.len()
-        }
-    }
 
     for (idx, signal) in ALL_SIGNALS.iter().enumerate() {
-        print!("{0: >#2} {1: <#8}", idx + 1, signal.name);
+        print!("{0: >#2} {1: <#2$}", idx, signal, name_width + 2);
-        //TODO: obtain max signal width here
-
         if (idx + 1) % 7 == 0 {
             println!();
         }
     }
+    println!()
 }
 
 fn print_signal(signal_name_or_value: &str) {
-    for signal in &ALL_SIGNALS {
+    for (value, &signal) in ALL_SIGNALS.iter().enumerate() {
-        if signal.name == signal_name_or_value
+        if signal == signal_name_or_value || (format!("SIG{}", signal)) == signal_name_or_value {
-            || (format!("SIG{}", signal.name)) == signal_name_or_value
+            println!("{}", value);
-        {
-            println!("{}", signal.value);
             exit!(EXIT_OK as i32)
-        } else if signal_name_or_value == signal.value.to_string() {
+        } else if signal_name_or_value == value.to_string() {
-            println!("{}", signal.name);
+            println!("{}", signal);
             exit!(EXIT_OK as i32)
         }
     }
@@ -165,8 +156,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();
+        pos += signal.len();
-        print!("{}", signal.name);
+        print!("{}", signal);
         if idx > 0 && pos > 73 {
             println!();
             pos = 0;

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=["process", "signals"] }
 uucore_procs = { version=">=0.0.5", package="uucore_procs", path="../../uucore_procs" }
 

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;
@@ -17,13 +17,13 @@ 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.";
 
 fn get_usage() -> String {
-    format!("{0} [OPTION]... [FILE]...", executable!())
+    format!("{0} [OPTION] DURATION COMMAND...", executable!())
 }
 
 const ERR_EXIT_STATUS: i32 = 125;
@@ -33,22 +33,22 @@ 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";
     pub static COMMAND: &str = "command";
-    pub static ARGS: &str = "args";
 }
 
 struct Config {
     foreground: bool,
-    kill_after: Duration,
+    kill_after: Option<Duration>,
     signal: usize,
     duration: Duration,
     preserve_status: bool,
+    verbose: bool,
 
-    command: String,
+    command: Vec<String>,
-    command_args: Vec<String>,
 }
 
 impl Config {
@@ -66,23 +66,22 @@ impl Config {
             _ => uucore::signals::signal_by_name_or_value("TERM").unwrap(),
         };
 
-        let kill_after: Duration = match options.value_of(options::KILL_AFTER) {
+        let kill_after = options
-            Some(time) => uucore::parse_time::from_str(time).unwrap(),
+            .value_of(options::KILL_AFTER)
-            None => Duration::new(0, 0),
+            .map(|time| uucore::parse_time::from_str(time).unwrap());
-        };
 
         let duration: Duration =
             uucore::parse_time::from_str(options.value_of(options::DURATION).unwrap()).unwrap();
 
         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: String = options.value_of(options::COMMAND).unwrap().to_string();
+        let command = options
-
+            .values_of(options::COMMAND)
-        let command_args: Vec<String> = match options.values_of(options::ARGS) {
+            .unwrap()
-            Some(values) => values.map(|x| x.to_owned()).collect(),
+            .map(String::from)
-            None => vec![],
+            .collect::<Vec<_>>();
-        };
 
         Config {
             foreground,
@@ -91,7 +90,7 @@ impl Config {
             duration,
             preserve_status,
             command,
-            command_args,
+            verbose,
         }
     }
 }
@@ -128,6 +127,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)
@@ -137,9 +142,7 @@ pub fn uumain(args: impl uucore::Args) -> i32 {
             Arg::with_name(options::COMMAND)
                 .index(2)
                 .required(true)
-        )
+                .multiple(true)
-        .arg(
-            Arg::with_name(options::ARGS).multiple(true)
         )
         .setting(AppSettings::TrailingVarArg);
 
@@ -148,31 +151,42 @@ 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,
         config.foreground,
         config.preserve_status,
+        config.verbose,
     )
 }
 
+/// 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(
-    cmdname: &str,
+    cmd: &[String],
-    args: &[String],
     duration: Duration,
     signal: usize,
-    kill_after: Duration,
+    kill_after: Option<Duration>,
     foreground: bool,
     preserve_status: bool,
+    verbose: bool,
 ) -> i32 {
     if !foreground {
         unsafe { libc::setpgid(0, 0) };
     }
-    let mut process = match Command::new(cmdname)
+    let mut process = match Command::new(&cmd[0])
-        .args(args)
+        .args(&cmd[1..])
         .stdin(Stdio::inherit())
         .stdout(Stdio::inherit())
         .stderr(Stdio::inherit())
@@ -190,10 +204,19 @@ fn timeout(
             }
         }
     };
+    unblock_sigchld();
     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));
+            if let Some(kill_after) = kill_after {
                 match process.wait_or_timeout(kill_after) {
                     Ok(Some(status)) => {
                         if preserve_status {
@@ -203,20 +226,23 @@ fn timeout(
                         }
                     }
                     Ok(None) => {
-                    if kill_after == Duration::new(0, 0) {
+                        if verbose {
-                        // XXX: this may not be right
+                            show_error!("sending signal KILL to command '{}'", cmd[0]);
-                        return 124;
                         }
                         return_if_err!(
                             ERR_EXIT_STATUS,
-                        process
+                            process.send_signal(
-                            .send_signal(uucore::signals::signal_by_name_or_value("KILL").unwrap())
+                                uucore::signals::signal_by_name_or_value("KILL").unwrap()
+                            )
                         );
                         return_if_err!(ERR_EXIT_STATUS, process.wait());
                         137
                     }
                     Err(_) => 124,
                 }
+            } else {
+                124
+            }
         }
         Err(_) => {
             return_if_err!(ERR_EXIT_STATUS, process.send_signal(signal));

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<Option<ExitStatus>>;
 }
 
@@ -106,6 +107,11 @@ impl ChildExt for Child {
     }
 
     fn wait_or_timeout(&mut self, timeout: Duration) -> io::Result<Option<ExitStatus>> {
+        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());
 

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,131 +24,10 @@ Linux Programmer's Manual
 */
 
 #[cfg(target_os = "linux")]
-pub static ALL_SIGNALS: [Signal<'static>; 31] = [
+pub static ALL_SIGNALS: [&str; 32] = [
-    Signal {
+    "EXIT", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "BUS", "FPE", "KILL", "USR1", "SEGV",
-        name: "HUP",
+    "USR2", "PIPE", "ALRM", "TERM", "STKFLT", "CHLD", "CONT", "STOP", "TSTP", "TTIN", "TTOU",
-        value: 1,
+    "URG", "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "POLL", "PWR", "SYS",
-    },
-    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,
-    },
 ];
 
 /*
@@ -198,131 +72,10 @@ No    Name         Default Action       Description
 */
 
 #[cfg(any(target_vendor = "apple", target_os = "freebsd"))]
-pub static ALL_SIGNALS: [Signal<'static>; 31] = [
+pub static ALL_SIGNALS: [&str; 32] = [
-    Signal {
+    "EXIT", "HUP", "INT", "QUIT", "ILL", "TRAP", "ABRT", "EMT", "FPE", "KILL", "BUS", "SEGV",
-        name: "HUP",
+    "SYS", "PIPE", "ALRM", "TERM", "URG", "STOP", "TSTP", "CONT", "CHLD", "TTIN", "TTOU", "IO",
-        value: 1,
+    "XCPU", "XFSZ", "VTALRM", "PROF", "WINCH", "INFO", "USR1", "USR2",
-    },
-    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 fn signal_by_name_or_value(signal_name_or_value: &str) -> Option<usize> {
@@ -335,56 +88,45 @@ pub fn signal_by_name_or_value(signal_name_or_value: &str) -> Option<usize> {
     }
     let signal_name = signal_name_or_value.trim_start_matches("SIG");
 
-    ALL_SIGNALS
+    ALL_SIGNALS.iter().position(|&s| s == signal_name)
-        .iter()
-        .find(|s| s.name == signal_name)
-        .map(|s| s.value)
 }
 
-#[inline(always)]
 pub fn is_signal(num: usize) -> bool {
-    // Named signals start at 1
+    num < ALL_SIGNALS.len()
-    num <= ALL_SIGNALS.len()
 }
 
-#[test]
+pub fn signal_name_by_value(signal_value: usize) -> Option<&'static str> {
-fn signals_all_contiguous() {
+    ALL_SIGNALS.get(signal_value).copied()
-    for (i, signal) in ALL_SIGNALS.iter().enumerate() {
-        assert_eq!(signal.value, i + 1);
-    }
-}
-
-#[test]
-fn signals_all_are_signal() {
-    for signal in &ALL_SIGNALS {
-        assert!(is_signal(signal.value));
-    }
 }
 
 #[test]
 fn signal_by_value() {
     assert_eq!(signal_by_name_or_value("0"), Some(0));
-    for signal in &ALL_SIGNALS {
+    for (value, _signal) in ALL_SIGNALS.iter().enumerate() {
-        assert_eq!(
+        assert_eq!(signal_by_name_or_value(&value.to_string()), Some(value));
-            signal_by_name_or_value(&signal.value.to_string()),
-            Some(signal.value)
-        );
     }
 }
 
 #[test]
 fn signal_by_short_name() {
-    for signal in &ALL_SIGNALS {
+    for (value, signal) in ALL_SIGNALS.iter().enumerate() {
-        assert_eq!(signal_by_name_or_value(signal.name), Some(signal.value));
+        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)),
+            signal_by_name_or_value(&format!("SIG{}", signal)),
-            Some(signal.value)
+            Some(value)
         );
     }
 }
+
+#[test]
+fn name() {
+    for (value, signal) in ALL_SIGNALS.iter().enumerate() {
+        assert_eq!(signal_name_by_value(value), Some(*signal));
+    }
+}

tests/by-util/test_timeout.rs

@@ -9,3 +9,39 @@ 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");
+}
+
+#[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'");
+    }
+}
+
+#[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();
+}