Skip to content

Add RawStatusCode #403

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

Closed
joboet opened this issue Jun 25, 2024 · 4 comments
Closed

Add RawStatusCode #403

joboet opened this issue Jun 25, 2024 · 4 comments
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api waiting-on-author

Comments

@joboet
Copy link
Member

joboet commented Jun 25, 2024

Proposal

Problem statement

rust-lang/rust#123196 will add limited process spawing support for UEFI. Unfortunately, the current ExitStatus API does not allow returning the pointer-sized status codes that UEFI reports as the return type of ExitStatus::code is i32.

Solution sketch

Similar to the RawOsError type introduced for a similar purpose, add a RawStatusCode type alias to std::process that aliases to i32 on every platform but UEFI.

// in std::process

#[cfg(target_os = "uefi")]
pub type RawStatusCode = usize;
#[cfg(not(target_os = "uefi")]
pub type RawStatusCode = i32;

Alternatives

  • Truncate the return codes
  • Add a separate, UEFI-specific method for getting codes

Links and related work

What happens now?

This issue contains an API change proposal (or ACP) and is part of the libs-api team feature lifecycle. Once this issue is filed, the libs-api team will review open proposals as capability becomes available. Current response times do not have a clear estimate, but may be up to several months.

Possible responses

The libs team may respond in various different ways. First, the team will consider the problem (this doesn't require any concrete solution or alternatives to have been proposed):

  • We think this problem seems worth solving, and the standard library might be the right place to solve it.
  • We think that this probably doesn't belong in the standard library.

Second, if there's a concrete solution:

  • We think this specific solution looks roughly right, approved, you or someone else should implement this. (Further review will still happen on the subsequent implementation PR.)
  • We're not sure this is the right solution, and the alternatives or other materials don't give us enough information to be sure about that. Here are some questions we have that aren't answered, or rough ideas about alternatives we'd want to see discussed.
@joboet joboet added api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api labels Jun 25, 2024
@scottmcm
Copy link
Member

scottmcm commented Jul 2, 2024

Is there any iteraction with https://doc.rust-lang.org/stable/std/process/struct.ExitCode.html that could make sense here? Since that type is itself about how the correct type for an exit code is not just an i32.

There's https://doc.rust-lang.org/stable/std/process/struct.ExitCode.html?#impl-ExitCodeExt-for-ExitCode for windows's DWORD exits; should there be a UEFI version of that taking usize?

@joboet
Copy link
Member Author

joboet commented Jul 3, 2024

Is there any iteraction with https://doc.rust-lang.org/stable/std/process/struct.ExitCode.html that could make sense here? Since that type is itself about how the correct type for an exit code is not just an i32.

The documentation says that

ExitCode is intended for terminating the currently running process [...] in contrast to ExitStatus, which represents the termination of a child process.

so I don't think it can be used here.

There's https://doc.rust-lang.org/stable/std/process/struct.ExitCode.html?#impl-ExitCodeExt-for-ExitCode for windows's DWORD exits; should there be a UEFI version of that taking usize?

Probably, but that's beside the point here.

@joboet
Copy link
Member Author

joboet commented Jul 9, 2024

Nominating so that this doesn't slip through the cracks (again):

@rustbot label +I-libs-api-nominated

@rustbot rustbot added the I-libs-api-nominated Indicates that an issue has been nominated for discussion during a team meeting. label Jul 9, 2024
@joshtriplett
Copy link
Member

We talked about this in today's @rust-lang/libs-api meeting.

We don't think we want a general RawStatusCode, not least of which because having a default of i32 on all platforms will make it hard to change this if another platform ever comes up, or for that matter, if we end up needing something different on a platform we currently have.

Instead, we'd like to see a std::os::uefi::EfiStatus type (or similar) added (which may in the future also want some methods), and an extension trait similar to https://doc.rust-lang.org/std/os/unix/process/trait.ExitStatusExt.html with methods from_raw and into_raw that use EfiStatus.

@Amanieu Amanieu added waiting-on-author and removed I-libs-api-nominated Indicates that an issue has been nominated for discussion during a team meeting. labels Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-change-proposal A proposal to add or alter unstable APIs in the standard libraries T-libs-api waiting-on-author
Projects
None yet
Development

No branches or pull requests

5 participants