Skip to content

Commit

Permalink
Rollup merge of rust-lang#82411 - ijackson:fix-exitstatus, r=dtolnay
Browse files Browse the repository at this point in the history
Fixes to ExitStatus and its docs

* On Unix, properly display every possible wait status (and don't panic on weird values)
* In the documentation, be clear and consistent about "exit status" vs "wait status".
  • Loading branch information
Dylan-DPC authored Mar 4, 2021
2 parents ba4ee48 + 4bb8425 commit ae2482e
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 11 deletions.
25 changes: 18 additions & 7 deletions library/std/src/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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());
/// ```
Expand Down Expand Up @@ -1368,11 +1368,17 @@ impl From<fs::File> 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)]
Expand Down Expand Up @@ -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")]
Expand All @@ -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
///
Expand Down
14 changes: 12 additions & 2 deletions library/std/src/sys/unix/ext/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,12 +188,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;

Expand Down Expand Up @@ -222,6 +230,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;
}
Expand Down
17 changes: 15 additions & 2 deletions library/std/src/sys/unix/process/process_unix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -527,9 +527,22 @@ impl fmt::Display for ExitStatus {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
if let Some(code) = self.code() {
write!(f, "exit code: {}", code)
} else if let Some(signal) = self.signal() {
if self.core_dumped() {
write!(f, "signal: {} (core dumped)", signal)
} else {
write!(f, "signal: {}", signal)
}
} else if let Some(signal) = self.stopped_signal() {
write!(f, "stopped (not terminated) by signal: {}", signal)
} else if self.continued() {
write!(f, "continued (WIFCONTINUED)")
} else {
let signal = self.signal().unwrap();
write!(f, "signal: {}", signal)
write!(f, "unrecognised wait status: {} {:#x}", self.0, self.0)
}
}
}

#[cfg(test)]
#[path = "process_unix/tests.rs"]
mod tests;
22 changes: 22 additions & 0 deletions library/std/src/sys/unix/process/process_unix/tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
#[test]
fn exitstatus_display_tests() {
// In practice this is the same on every Unix.
// If some weird platform turns out to be different, and this test fails, use #[cfg].
use crate::os::unix::process::ExitStatusExt;
use crate::process::ExitStatus;

let t = |v, s| assert_eq!(s, format!("{}", <ExitStatus as ExitStatusExt>::from_raw(v)));

t(0x0000f, "signal: 15");
t(0x0008b, "signal: 11 (core dumped)");
t(0x00000, "exit code: 0");
t(0x0ff00, "exit code: 255");
t(0x0137f, "stopped (not terminated) by signal: 19");
t(0x0ffff, "continued (WIFCONTINUED)");

// Testing "unrecognised wait status" is hard because the wait.h macros typically
// assume that the value came from wait and isn't mad. With the glibc I have here
// this works:
#[cfg(target_env = "gnu")]
t(0x000ff, "unrecognised wait status: 255 0xff");
}

0 comments on commit ae2482e

Please sign in to comment.