timeout: cleanup return values (#9576)

- remove "WaitingFailed" which is a duplicate of "CommandTimedOut"
- replace hard-coded values 126 and 127 with enum values, remove TODO
- fix misleading comment. we DO return CommandTimedOut even when preserve-status is not specified
- add tests for exit values 126 and 127

Signed-off-by: Etienne Cordonnier <ecordonnier@snap.com>

src/uu/timeout/src/status.rs

@@ -19,18 +19,21 @@ use uucore::error::UError;
 /// assert_eq!(i32::from(ExitStatus::CommandTimedOut), 124);
 /// ```
 pub(crate) enum ExitStatus {
-    /// When the child process times out and `--preserve-status` is not specified.
+    /// When the child process times out.
     CommandTimedOut,
 
     /// When `timeout` itself fails.
     TimeoutFailed,
 
+    /// When command is found but cannot be invoked (permission denied, etc.).
+    CannotInvoke,
+
+    /// When command cannot be found.
+    CommandNotFound,
+
     /// 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,
-
     /// When `SIGTERM` signal received.
     Terminated,
 }
@@ -40,8 +43,9 @@ impl From<ExitStatus> for i32 {
         match exit_status {
             ExitStatus::CommandTimedOut => 124,
             ExitStatus::TimeoutFailed => 125,
+            ExitStatus::CannotInvoke => 126,
+            ExitStatus::CommandNotFound => 127,
             ExitStatus::SignalSent(s) => 128 + s as Self,
-            ExitStatus::WaitingFailed => 124,
             ExitStatus::Terminated => 143,
         }
     }

src/uu/timeout/src/timeout.rs

@@ -275,7 +275,7 @@ fn wait_or_kill_process(
             process.wait()?;
             Ok(ExitStatus::SignalSent(signal).into())
         }
-        Err(_) => Ok(ExitStatus::WaitingFailed.into()),
+        Err(_) => Ok(ExitStatus::CommandTimedOut.into()),
     }
 }
 
@@ -305,7 +305,6 @@ fn preserve_signal_info(signal: libc::c_int) -> libc::c_int {
     signal
 }
 
-/// TODO: Improve exit codes, and make them consistent with the GNU Coreutils exit codes.
 fn timeout(
     cmd: &[String],
     duration: Duration,
@@ -328,12 +327,10 @@ fn timeout(
         .stderr(Stdio::inherit())
         .spawn()
         .map_err(|err| {
-            let status_code = if err.kind() == ErrorKind::NotFound {
-                // FIXME: not sure which to use
-                127
-            } else {
-                // FIXME: this may not be 100% correct...
-                126
+            let status_code = match err.kind() {
+                ErrorKind::NotFound => ExitStatus::CommandNotFound.into(),
+                ErrorKind::PermissionDenied => ExitStatus::CannotInvoke.into(),
+                _ => ExitStatus::CannotInvoke.into(),
             };
             USimpleError::new(
                 status_code,

tests/by-util/test_timeout.rs

@@ -223,3 +223,18 @@ fn test_terminate_child_on_receiving_terminate() {
         .code_is(143)
         .stdout_contains("child received TERM");
 }
+
+#[test]
+fn test_command_not_found() {
+    // Test exit code 127 when command doesn't exist
+    new_ucmd!()
+        .args(&["1", "/this/command/definitely/does/not/exist"])
+        .fails_with_code(127);
+}
+
+#[test]
+fn test_command_cannot_invoke() {
+    // Test exit code 126 when command exists but cannot be invoked
+    // Try to execute a directory (should give permission denied or similar)
+    new_ucmd!().args(&["1", "/"]).fails_with_code(126);
+}