Skip to content

Commit e893089

Browse files
ijacksonijackson
committed
Provide ExitStatusError
Closes rust-lang#73125 This is in pursuance of Issue rust-lang#73127 Consider adding #[must_use] to std::process::ExitStatus In MR rust-lang#81452 Add #[must_use] to [...] process::ExitStatus we concluded that the existing arrangements in are too awkward so adding that #[must_use] is blocked on improving the ergonomics. I wrote a mini-RFC-style discusion of the approach in rust-lang#73125 (comment) Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
1 parent b50c1bb commit e893089

File tree

6 files changed

+227
-13
lines changed

6 files changed

+227
-13
lines changed

‎library/std/src/process.rs

+135-3
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ use crate::ffi::OsStr;
110110
use crate::fmt;
111111
use crate::fs;
112112
use crate::io::{self, Initializer, IoSlice, IoSliceMut};
113+
use crate::num::NonZeroI32;
113114
use crate::path::Path;
114115
use crate::str;
115116
use crate::sys::pipe::{read2, AnonPipe};
@@ -1387,8 +1388,8 @@ impl From<fs::File> for Stdio {
13871388
/// An `ExitStatus` represents every possible disposition of a process. On Unix this
13881389
/// is the **wait status**. It is *not* simply an *exit status* (a value passed to `exit`).
13891390
///
1390-
/// For proper error reporting of failed processes, print the value of `ExitStatus` using its
1391-
/// implementation of [`Display`](crate::fmt::Display).
1391+
/// For proper error reporting of failed processes, print the value of `ExitStatus` or
1392+
/// `ExitStatusError` using their implementations of [`Display`](crate::fmt::Display).
13921393
///
13931394
/// [`status`]: Command::status
13941395
/// [`wait`]: Child::wait
@@ -1401,6 +1402,29 @@ pub struct ExitStatus(imp::ExitStatus);
14011402
impl crate::sealed::Sealed for ExitStatus {}
14021403

14031404
impl ExitStatus {
1405+
/// Was termination successful? Returns a `Result`.
1406+
///
1407+
/// # Examples
1408+
///
1409+
/// ```
1410+
/// #![feature(exit_status_error)]
1411+
/// # if cfg!(unix) {
1412+
/// use std::process::Command;
1413+
///
1414+
/// let status = Command::new("ls")
1415+
/// .arg("/dev/nonexistent")
1416+
/// .status()
1417+
/// .expect("ls could not be executed");
1418+
///
1419+
/// println!("ls: {}", status);
1420+
/// status.exit_ok().expect_err("/dev/nonexistent could be listed!");
1421+
/// # } // cfg!(unix)
1422+
/// ```
1423+
#[unstable(feature = "exit_status_error", issue = "84908")]
1424+
pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
1425+
self.0.exit_ok().map_err(ExitStatusError)
1426+
}
1427+
14041428
/// Was termination successful? Signal termination is not considered a
14051429
/// success, and success is defined as a zero exit status.
14061430
///
@@ -1422,7 +1446,7 @@ impl ExitStatus {
14221446
/// ```
14231447
#[stable(feature = "process", since = "1.0.0")]
14241448
pub fn success(&self) -> bool {
1425-
self.0.success()
1449+
self.0.exit_ok().is_ok()
14261450
}
14271451

14281452
/// Returns the exit code of the process, if any.
@@ -1476,6 +1500,114 @@ impl fmt::Display for ExitStatus {
14761500
}
14771501
}
14781502

1503+
/// Describes the result of a process after it has failed
1504+
///
1505+
/// Produced by the [`.exit_ok`](ExitStatus::exit_ok) method on [`ExitStatus`].
1506+
///
1507+
/// # Examples
1508+
///
1509+
/// ```
1510+
/// #![feature(exit_status_error)]
1511+
/// # if cfg!(unix) {
1512+
/// use std::process::{Command, ExitStatusError};
1513+
///
1514+
/// fn run(cmd: &str) -> Result<(),ExitStatusError> {
1515+
/// Command::new(cmd).status().unwrap().exit_ok()?;
1516+
/// Ok(())
1517+
/// }
1518+
///
1519+
/// run("true").unwrap();
1520+
/// run("false").unwrap_err();
1521+
/// # } // cfg!(unix)
1522+
/// ```
1523+
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
1524+
#[unstable(feature = "exit_status_error", issue = "84908")]
1525+
// The definition of imp::ExitStatusError should ideally be such that
1526+
// Result<(), imp::ExitStatusError> has an identical representation to imp::ExitStatus.
1527+
pub struct ExitStatusError(imp::ExitStatusError);
1528+
1529+
#[unstable(feature = "exit_status_error", issue = "84908")]
1530+
impl ExitStatusError {
1531+
/// Reports the exit code, if applicable, from an `ExitStatusError`.
1532+
///
1533+
/// In Unix terms the return value is the **exit status**: the value passed to `exit`, if the
1534+
/// process finished by calling `exit`. Note that on Unix the exit status is truncated to 8
1535+
/// bits, and that values that didn't come from a program's call to `exit` may be invented the
1536+
/// runtime system (often, for example, 255, 254, 127 or 126).
1537+
///
1538+
/// On Unix, this will return `None` if the process was terminated by a signal. If you want to
1539+
/// handle such situations specially, consider using
1540+
/// [`ExitStatusExt`](crate::os::unix::process::ExitStatusExt) (possibly after getting the
1541+
/// general `ExitStatus` by using [`status()`](ExitStatusError::status).
1542+
///
1543+
/// If the process finished by calling `exit` with a nonzero value, this will return
1544+
/// that exit status.
1545+
///
1546+
/// If the error was something else, it will return `None`.
1547+
///
1548+
/// If the process exited successfully (ie, by calling `exit(0)`), there is no
1549+
/// `ExitStatusError`. So the return value from `ExitStatusError::code()` is always nonzero.
1550+
///
1551+
/// # Examples
1552+
///
1553+
/// ```
1554+
/// #![feature(exit_status_error)]
1555+
/// # #[cfg(unix)] {
1556+
/// use std::process::Command;
1557+
///
1558+
/// let bad = Command::new("false").status().unwrap().exit_ok().unwrap_err();
1559+
/// assert_eq!(bad.code(), Some(1));
1560+
/// # } // #[cfg(unix)]
1561+
/// ```
1562+
pub fn code(&self) -> Option<i32> {
1563+
self.code_nonzero().map(Into::into)
1564+
}
1565+
1566+
/// Reports the exit code, if applicable, from an `ExitStatusError`, as a `NonZero`
1567+
///
1568+
/// This is exaclty like [`code()`](Self::code), except that it returns a `NonZeroI32`.
1569+
///
1570+
/// Plain `code`, returning a plain integer, is provided because is is often more convenient.
1571+
/// The returned value from `code()` is indeed also nonzero; use `code_nonzero()` when you want
1572+
/// a type-level guarantee of nonzeroness.
1573+
///
1574+
/// # Examples
1575+
///
1576+
/// ```
1577+
/// #![feature(exit_status_error)]
1578+
/// # if cfg!(unix) {
1579+
/// use std::convert::TryFrom;
1580+
/// use std::num::NonZeroI32;
1581+
/// use std::process::Command;
1582+
///
1583+
/// let bad = Command::new("false").status().unwrap().exit_ok().unwrap_err();
1584+
/// assert_eq!(bad.code_nonzero().unwrap(), NonZeroI32::try_from(1).unwrap());
1585+
/// # } // cfg!(unix)
1586+
/// ```
1587+
pub fn code_nonzero(&self) -> Option<NonZeroI32> {
1588+
self.0.code()
1589+
}
1590+
1591+
/// Converts an `ExitStatusError` (back) to an `ExitStatus`.
1592+
pub fn into_status(&self) -> ExitStatus {
1593+
ExitStatus(self.0.into())
1594+
}
1595+
}
1596+
1597+
#[unstable(feature = "exit_status_error", issue = "84908")]
1598+
impl Into<ExitStatus> for ExitStatusError {
1599+
fn into(self) -> ExitStatus {
1600+
ExitStatus(self.0.into())
1601+
}
1602+
}
1603+
1604+
#[unstable(feature = "exit_status_error", issue = "84908")]
1605+
impl fmt::Display for ExitStatusError {
1606+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
1607+
self.into_status().fmt(f)
1608+
}
1609+
}
1610+
14791611
/// This type represents the status code a process can return to its
14801612
/// parent under normal termination.
14811613
///

‎library/std/src/sys/unix/process/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
pub use self::process_common::{Command, CommandArgs, ExitCode, Stdio, StdioPipes};
2-
pub use self::process_inner::{ExitStatus, Process};
2+
pub use self::process_inner::{ExitStatus, ExitStatusError, Process};
33
pub use crate::ffi::OsString as EnvKey;
44
pub use crate::sys_common::process::CommandEnvs;
55

‎library/std/src/sys/unix/process/process_fuchsia.rs

+23-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
use crate::convert::TryInto;
1+
use crate::convert::{TryFrom, TryInto};
22
use crate::fmt;
33
use crate::io;
44
use crate::mem;
5+
use crate::num::{NonZeroI32, NonZeroI64};
56
use crate::ptr;
67

78
use crate::sys::process::process_common::*;
@@ -236,8 +237,11 @@ impl Process {
236237
pub struct ExitStatus(i64);
237238

238239
impl ExitStatus {
239-
pub fn success(&self) -> bool {
240-
self.code() == Some(0)
240+
pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
241+
match NonZeroI64::try_from(self.0) {
242+
/* was nonzero */ Ok(failure) => Err(ExitStatusError(failure)),
243+
/* was zero, couldn't convert */ Err(_) => Ok(()),
244+
}
241245
}
242246

243247
pub fn code(&self) -> Option<i32> {
@@ -306,3 +310,19 @@ impl fmt::Display for ExitStatus {
306310
write!(f, "exit code: {}", self.0)
307311
}
308312
}
313+
314+
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
315+
pub struct ExitStatusError(NonZeroI64);
316+
317+
impl Into<ExitStatus> for ExitStatusError {
318+
fn into(self) -> ExitStatus {
319+
ExitStatus(self.0.into())
320+
}
321+
}
322+
323+
impl ExitStatusError {
324+
pub fn code(self) -> Option<NonZeroI32> {
325+
// fixme: affected by the same bug as ExitStatus::code()
326+
ExitStatus(self.0.into()).code().map(|st| st.try_into().unwrap())
327+
}
328+
}

‎library/std/src/sys/unix/process/process_unix.rs

+28-3
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
use crate::convert::TryInto;
1+
use crate::convert::{TryFrom, TryInto};
22
use crate::fmt;
33
use crate::io::{self, Error, ErrorKind};
44
use crate::mem;
5+
use crate::num::NonZeroI32;
6+
use crate::os::raw::NonZero_c_int;
57
use crate::ptr;
68
use crate::sys;
79
use crate::sys::cvt;
@@ -490,8 +492,16 @@ impl ExitStatus {
490492
libc::WIFEXITED(self.0)
491493
}
492494

493-
pub fn success(&self) -> bool {
494-
self.code() == Some(0)
495+
pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
496+
// This assumes that WIFEXITED(status) && WEXITSTATUS==0 corresponds to status==0. This is
497+
// true on all actual versios of Unix, is widely assumed, and is specified in SuS
498+
// https://pubs.opengroup.org/onlinepubs/9699919799/functions/wait.html . If it is not
499+
// true for a platform pretending to be Unix, the tests (our doctests, and also
500+
// procsss_unix/tests.rs) will spot it. `ExitStatusError::code` assumes this too.
501+
match NonZero_c_int::try_from(self.0) {
502+
/* was nonzero */ Ok(failure) => Err(ExitStatusError(failure)),
503+
/* was zero, couldn't convert */ Err(_) => Ok(()),
504+
}
495505
}
496506

497507
pub fn code(&self) -> Option<i32> {
@@ -546,6 +556,21 @@ impl fmt::Display for ExitStatus {
546556
}
547557
}
548558

559+
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
560+
pub struct ExitStatusError(NonZero_c_int);
561+
562+
impl Into<ExitStatus> for ExitStatusError {
563+
fn into(self) -> ExitStatus {
564+
ExitStatus(self.0.into())
565+
}
566+
}
567+
568+
impl ExitStatusError {
569+
pub fn code(self) -> Option<NonZeroI32> {
570+
ExitStatus(self.0.into()).code().map(|st| st.try_into().unwrap())
571+
}
572+
}
573+
549574
#[cfg(test)]
550575
#[path = "process_unix/tests.rs"]
551576
mod tests;

‎library/std/src/sys/unsupported/process.rs

+17-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use crate::ffi::OsStr;
22
use crate::fmt;
33
use crate::io;
44
use crate::marker::PhantomData;
5+
use crate::num::NonZeroI32;
56
use crate::path::Path;
67
use crate::sys::fs::File;
78
use crate::sys::pipe::AnonPipe;
@@ -97,7 +98,7 @@ impl fmt::Debug for Command {
9798
pub struct ExitStatus(!);
9899

99100
impl ExitStatus {
100-
pub fn success(&self) -> bool {
101+
pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
101102
self.0
102103
}
103104

@@ -134,6 +135,21 @@ impl fmt::Display for ExitStatus {
134135
}
135136
}
136137

138+
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
139+
pub struct ExitStatusError(ExitStatus);
140+
141+
impl Into<ExitStatus> for ExitStatusError {
142+
fn into(self) -> ExitStatus {
143+
self.0.0
144+
}
145+
}
146+
147+
impl ExitStatusError {
148+
pub fn code(self) -> Option<NonZeroI32> {
149+
self.0.0
150+
}
151+
}
152+
137153
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
138154
pub struct ExitCode(bool);
139155

‎library/std/src/sys/windows/process.rs

+23-2
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,20 @@ mod tests;
55

66
use crate::borrow::Borrow;
77
use crate::collections::BTreeMap;
8+
use crate::convert::{TryFrom, TryInto};
89
use crate::env;
910
use crate::env::split_paths;
1011
use crate::ffi::{OsStr, OsString};
1112
use crate::fmt;
1213
use crate::fs;
1314
use crate::io::{self, Error, ErrorKind};
1415
use crate::mem;
16+
use crate::num::NonZeroI32;
1517
use crate::os::windows::ffi::OsStrExt;
1618
use crate::path::Path;
1719
use crate::ptr;
1820
use crate::sys::c;
21+
use crate::sys::c::NonZeroDWORD;
1922
use crate::sys::cvt;
2023
use crate::sys::fs::{File, OpenOptions};
2124
use crate::sys::handle::Handle;
@@ -376,8 +379,11 @@ impl Process {
376379
pub struct ExitStatus(c::DWORD);
377380

378381
impl ExitStatus {
379-
pub fn success(&self) -> bool {
380-
self.0 == 0
382+
pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
383+
match NonZeroDWORD::try_from(self.0) {
384+
/* was nonzero */ Ok(failure) => Err(ExitStatusError(failure)),
385+
/* was zero, couldn't convert */ Err(_) => Ok(()),
386+
}
381387
}
382388
pub fn code(&self) -> Option<i32> {
383389
Some(self.0 as i32)
@@ -406,6 +412,21 @@ impl fmt::Display for ExitStatus {
406412
}
407413
}
408414

415+
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
416+
pub struct ExitStatusError(c::NonZeroDWORD);
417+
418+
impl Into<ExitStatus> for ExitStatusError {
419+
fn into(self) -> ExitStatus {
420+
ExitStatus(self.0.into())
421+
}
422+
}
423+
424+
impl ExitStatusError {
425+
pub fn code(self) -> Option<NonZeroI32> {
426+
Some((u32::from(self.0) as i32).try_into().unwrap())
427+
}
428+
}
429+
409430
#[derive(PartialEq, Eq, Clone, Copy, Debug)]
410431
pub struct ExitCode(c::DWORD);
411432

0 commit comments

Comments
 (0)