From 4bb8425af60eb673a932c90ee8d1b5f24c13a34e Mon Sep 17 00:00:00 2001 From: Ian Jackson Date: Mon, 22 Feb 2021 15:26:19 +0000 Subject: [PATCH] ExitStatus: Improve documentation re wait status vs exit status The use of `ExitStatus` as the Rust type name for a Unix *wait status*, not an *exit status*, is very confusing, but sadly probably too late to change. This area is confusing enough in Unix already (and many programmers are already confuxed). We can at least document it. I chose *not* to mention the way shells like to exit with signal numbers, thus turning signal numbers into exit statuses. This is only relevant for Rust programs using `std::process` if they run shells. Signed-off-by: Ian Jackson --- library/std/src/process.rs | 25 ++++++++++++++++++------- library/std/src/sys/unix/ext/process.rs | 14 ++++++++++++-- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/library/std/src/process.rs b/library/std/src/process.rs index 6480e654c55f0..15ac9e402c589 100644 --- a/library/std/src/process.rs +++ b/library/std/src/process.rs @@ -885,7 +885,7 @@ impl Command { } /// Executes a command as a child process, waiting for it to finish and - /// collecting its exit status. + /// collecting its status. /// /// By default, stdin, stdout and stderr are inherited from the parent. /// @@ -899,7 +899,7 @@ impl Command { /// .status() /// .expect("failed to execute process"); /// - /// println!("process exited with: {}", status); + /// println!("process finished with: {}", status); /// /// assert!(status.success()); /// ``` @@ -1368,11 +1368,17 @@ impl From for Stdio { /// Describes the result of a process after it has terminated. /// -/// This `struct` is used to represent the exit status of a child process. +/// This `struct` is used to represent the exit status or other termination of a child process. /// Child processes are created via the [`Command`] struct and their exit /// status is exposed through the [`status`] method, or the [`wait`] method /// of a [`Child`] process. /// +/// An `ExitStatus` represents every possible disposition of a process. On Unix this +/// is the **wait status**. It is *not* simply an *exit status* (a value passed to `exit`). +/// +/// For proper error reporting of failed processes, print the value of `ExitStatus` using its +/// implementation of [`Display`](crate::fmt::Display). +/// /// [`status`]: Command::status /// [`wait`]: Child::wait #[derive(PartialEq, Eq, Clone, Copy, Debug)] @@ -1400,7 +1406,7 @@ impl ExitStatus { /// if status.success() { /// println!("'projects/' directory created"); /// } else { - /// println!("failed to create 'projects/' directory"); + /// println!("failed to create 'projects/' directory: {}", status); /// } /// ``` #[stable(feature = "process", since = "1.0.0")] @@ -1410,9 +1416,14 @@ impl ExitStatus { /// Returns the exit code of the process, if any. /// - /// On Unix, this will return `None` if the process was terminated - /// by a signal; `std::os::unix` provides an extension trait for - /// extracting the signal and other details from the `ExitStatus`. + /// In Unix terms the return value is the **exit status**: the value passed to `exit`, if the + /// process finished by calling `exit`. Note that on Unix the exit status is truncated to 8 + /// bits, and that values that didn't come from a program's call to `exit` may be invented the + /// runtime system (often, for example, 255, 254, 127 or 126). + /// + /// On Unix, this will return `None` if the process was terminated by a signal. + /// [`ExitStatusExt`](crate::os::unix::process::ExitStatusExt) is an + /// extension trait for extracting any such signal, and other details, from the `ExitStatus`. /// /// # Examples /// diff --git a/library/std/src/sys/unix/ext/process.rs b/library/std/src/sys/unix/ext/process.rs index 7559c1f1d9e29..6dc2033289a4a 100644 --- a/library/std/src/sys/unix/ext/process.rs +++ b/library/std/src/sys/unix/ext/process.rs @@ -186,12 +186,20 @@ impl CommandExt for process::Command { /// Unix-specific extensions to [`process::ExitStatus`]. /// +/// On Unix, `ExitStatus` **does not necessarily represent an exit status**, as passed to the +/// `exit` system call or returned by [`ExitStatus::code()`](crate::process::ExitStatus::code). +/// It represents **any wait status**, as returned by one of the `wait` family of system calls. +/// +/// This is because a Unix wait status (a Rust `ExitStatus`) can represent a Unix exit status, but +/// can also represent other kinds of process event. +/// /// This trait is sealed: it cannot be implemented outside the standard library. /// This is so that future additional methods are not breaking changes. #[stable(feature = "rust1", since = "1.0.0")] pub trait ExitStatusExt: Sealed { - /// Creates a new `ExitStatus` from the raw underlying `i32` return value of - /// a process. + /// Creates a new `ExitStatus` from the raw underlying integer status value from `wait` + /// + /// The value should be a **wait status, not an exit status**. #[stable(feature = "exit_status_from", since = "1.12.0")] fn from_raw(raw: i32) -> Self; @@ -220,6 +228,8 @@ pub trait ExitStatusExt: Sealed { fn continued(&self) -> bool; /// Returns the underlying raw `wait` status. + /// + /// The returned integer is a **wait status, not an exit status**. #[unstable(feature = "unix_process_wait_more", issue = "80695")] fn into_raw(self) -> i32; }