From 2d390233fef14434887db76ff388530df24a2e02 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 11 Mar 2022 22:33:25 -0500 Subject: [PATCH 1/2] timeout: add ExitStatus enumeration Add the `ExitStatus` enumeration in a new module `status.rs` to contain all the magic numbers in the `timeout` utility. --- src/uu/timeout/src/status.rs | 51 +++++++++++++++++++++++++++++++++++ src/uu/timeout/src/timeout.rs | 25 +++++++++-------- 2 files changed, 65 insertions(+), 11 deletions(-) create mode 100644 src/uu/timeout/src/status.rs diff --git a/src/uu/timeout/src/status.rs b/src/uu/timeout/src/status.rs new file mode 100644 index 000000000..9a8558954 --- /dev/null +++ b/src/uu/timeout/src/status.rs @@ -0,0 +1,51 @@ +// * This file is part of the uutils coreutils package. +// * +// * For the full copyright and license information, please view the LICENSE +// * file that was distributed with this source code. +//! Exit status codes produced by `timeout`. +use std::convert::From; +use uucore::error::UError; + +/// Enumerates the exit statuses produced by `timeout`. +/// +/// Use [`Into::into`] (or [`From::from`]) to convert an enumeration +/// member into a numeric status code. You can also convert into a +/// [`UError`]. +/// +/// # Examples +/// +/// Convert into an [`i32`]: +/// +/// ```rust,ignore +/// assert_eq!(i32::from(ExitStatus::CommandTimedOut), 124); +/// ``` +pub(crate) enum ExitStatus { + /// When the child process times out and `--preserve-status` is not specified. + CommandTimedOut, + + /// When `timeout` itself fails. + TimeoutFailed, + + /// When a signal is sent to the child process or `timeout` itself. + SignalSent(usize), + + /// When there is a failure while waiting for the child process to terminate. + WaitingFailed, +} + +impl From for i32 { + fn from(exit_status: ExitStatus) -> Self { + match exit_status { + ExitStatus::CommandTimedOut => 124, + ExitStatus::TimeoutFailed => 125, + ExitStatus::SignalSent(s) => 128 + s as Self, + ExitStatus::WaitingFailed => 124, + } + } +} + +impl From for Box { + fn from(exit_status: ExitStatus) -> Self { + Box::from(i32::from(exit_status)) + } +} diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index d90907fae..f02f0a0a9 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -6,12 +6,14 @@ // * file that was distributed with this source code. // spell-checker:ignore (ToDO) tstr sigstr cmdname setpgid sigchld +mod status; #[macro_use] extern crate uucore; extern crate clap; +use crate::status::ExitStatus; use clap::{crate_version, App, AppSettings, Arg}; use std::io::ErrorKind; use std::process::{Child, Command, Stdio}; @@ -25,8 +27,6 @@ use uucore::{format_usage, InvalidEncodingHandling}; static ABOUT: &str = "Start COMMAND, and kill it if still running after DURATION."; const USAGE: &str = "{} [OPTION] DURATION COMMAND..."; -const ERR_EXIT_STATUS: i32 = 125; - pub mod options { pub static FOREGROUND: &str = "foreground"; pub static KILL_AFTER: &str = "kill-after"; @@ -217,7 +217,7 @@ fn wait_or_kill_process( if preserve_status { Ok(status.code().unwrap_or_else(|| status.signal().unwrap())) } else { - Ok(124) + Ok(ExitStatus::TimeoutFailed.into()) } } Ok(None) => { @@ -225,9 +225,9 @@ fn wait_or_kill_process( report_if_verbose(signal, cmd, verbose); process.send_signal(signal)?; process.wait()?; - Ok(137) + Ok(ExitStatus::SignalSent(signal).into()) } - Err(_) => Ok(124), + Err(_) => Ok(ExitStatus::WaitingFailed.into()), } } @@ -282,7 +282,7 @@ fn timeout( report_if_verbose(signal, &cmd[0], verbose); process.send_signal(signal)?; match kill_after { - None => Err(124.into()), + None => Err(ExitStatus::CommandTimedOut.into()), Some(kill_after) => { match wait_or_kill_process( process, @@ -292,7 +292,10 @@ fn timeout( verbose, ) { Ok(status) => Err(status.into()), - Err(e) => Err(USimpleError::new(ERR_EXIT_STATUS, format!("{}", e))), + Err(e) => Err(USimpleError::new( + ExitStatus::TimeoutFailed.into(), + format!("{}", e), + )), } } } @@ -301,10 +304,10 @@ fn timeout( // We're going to return ERR_EXIT_STATUS regardless of // whether `send_signal()` succeeds or fails, so just // ignore the return value. - process - .send_signal(signal) - .map_err(|e| USimpleError::new(ERR_EXIT_STATUS, format!("{}", e)))?; - Err(ERR_EXIT_STATUS.into()) + process.send_signal(signal).map_err(|e| { + USimpleError::new(ExitStatus::TimeoutFailed.into(), format!("{}", e)) + })?; + Err(ExitStatus::TimeoutFailed.into()) } } } From 1aa6fd14680772e6a140d3c236a549b5e4014348 Mon Sep 17 00:00:00 2001 From: Jeffrey Finkelstein Date: Fri, 11 Mar 2022 22:40:47 -0500 Subject: [PATCH 2/2] timeout: fix bug in --preserve-status mode Fix a bug where `timeout --preserve-status` was not correctly preserving the status code of the child process if it timed out. When that happens, the status code of the child process is considered to be the signal number (in this case, `SIGTERM`). The exit status of `timeout` is then 128 plus the numeric code associated with `SIGTERM`. --- src/uu/timeout/src/timeout.rs | 8 +++++++- tests/by-util/test_timeout.rs | 11 +++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/uu/timeout/src/timeout.rs b/src/uu/timeout/src/timeout.rs index f02f0a0a9..3da0bcd2a 100644 --- a/src/uu/timeout/src/timeout.rs +++ b/src/uu/timeout/src/timeout.rs @@ -282,7 +282,13 @@ fn timeout( report_if_verbose(signal, &cmd[0], verbose); process.send_signal(signal)?; match kill_after { - None => Err(ExitStatus::CommandTimedOut.into()), + None => { + if preserve_status { + Err(ExitStatus::SignalSent(signal).into()) + } else { + Err(ExitStatus::CommandTimedOut.into()) + } + } Some(kill_after) => { match wait_or_kill_process( process, diff --git a/tests/by-util/test_timeout.rs b/tests/by-util/test_timeout.rs index 5db91a44d..b9c8a59ed 100644 --- a/tests/by-util/test_timeout.rs +++ b/tests/by-util/test_timeout.rs @@ -53,3 +53,14 @@ fn test_command_empty_args() { .fails() .stderr_contains("timeout: empty string"); } + +#[test] +fn test_preserve_status() { + new_ucmd!() + .args(&["--preserve-status", ".1", "sleep", "10"]) + .fails() + // 128 + SIGTERM = 128 + 15 + .code_is(128 + 15) + .no_stderr() + .no_stdout(); +}