Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExitCode::success() returns false when exit code is 0 #23

Open
believeinlain opened this issue May 6, 2024 · 1 comment
Open

ExitCode::success() returns false when exit code is 0 #23

believeinlain opened this issue May 6, 2024 · 1 comment
Assignees

Comments

@believeinlain
Copy link

I've encountered an error where memfd_exec::process::ExitStatus::success returns false when the execution was successful and returned with status code 0.

Minimal example to reproduce:

use memfd_exec::MemFdExecutable;

fn main() {
    let code = include_bytes!("/usr/bin/echo");

    let exit_status = MemFdExecutable::new("echo", code)
        .arg("Hello")
        .status()
        .expect("failed to execute process");
    // Exit status is 0
    println!("exit status {exit_status:?}");
    // Success fails
    assert!(exit_status.success());
}

It looks like the issue is in ExitStatus::exit_ok:

/// Was termination successful? Returns a Result.
pub fn exit_ok(&self) -> Result<()> {
    // This assumes that WIFEXITED(status) && WEXITSTATUS==0 corresponds to status==0.  This is
    // true on all actual versions of Unix, is widely assumed, and is specified in SuS
    // https://pubs.opengroup.org/onlinepubs/9699919799/functions/wait.html .  If it is not
    // true for a platform pretending to be Unix, the tests (our doctests, and also
    // procsss_unix/tests.rs) will spot it.  `ExitStatusError::code` assumes this too.
    #[allow(clippy::useless_conversion)]
    match c_int::try_from(self.0) {
        /* was nonzero */
        Ok(failure) => Err(Error::new(
            std::io::ErrorKind::Other,
            format!("process exited with status {}", failure),
        )),
        /* was zero, couldn't convert */
        Err(_) => Ok(()),
    }
}

This assumes that c_int::try_from(self.0) will fail if self.0 == 0, but a c_int can be 0 and the conversion always succeeds.

The standard library implementation (for std::sys::pal::unix::process::process_inner::ExitStatus) is:

pub fn exit_ok(&self) -> Result<(), ExitStatusError> {
    // This assumes that WIFEXITED(status) && WEXITSTATUS==0 corresponds to status==0. This is
    // true on all actual versions of Unix, is widely assumed, and is specified in SuS
    // https://pubs.opengroup.org/onlinepubs/9699919799/functions/wait.html. If it is not
    // true for a platform pretending to be Unix, the tests (our doctests, and also
    // process_unix/tests.rs) will spot it. `ExitStatusError::code` assumes this too.
    match NonZero::try_from(self.0) {
        /* was nonzero */ Ok(failure) => Err(ExitStatusError(failure)),
        /* was zero, couldn't convert */ Err(_) => Ok(()),
    }
}

Which makes sense, because NonZero actually cannot be 0, unlike c_int.

@novafacing novafacing self-assigned this May 7, 2024
@novafacing
Copy link
Owner

Interesting, I actually copied most of the code from std so I wonder if this used to be a bug there too :) I'll fix this soonish!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants