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

fix compilation on uefi (with std feature enabled) or i686 #566

Merged
merged 4 commits into from
Dec 18, 2024

Conversation

usamoi
Copy link
Contributor

@usamoi usamoi commented Dec 18, 2024

According to https://uefi.org/specs/UEFI/2.10/Apx_D_Status_Codes.html, there is no more space for internal errors, so use a wider integer on UEFI.

@newpavlov
Copy link
Member

newpavlov commented Dec 18, 2024

I don't understand why we need the Error changes. The UEFI status codes are not relevant for the RDRAND backend.

Also please add a build-only CI job for this target, see here.

@usamoi
Copy link
Contributor Author

usamoi commented Dec 18, 2024

@newpavlov If we use std on uefi targets, code does not compile since RawOsError on uefi is usize: https://github.com/rust-lang/rust/blob/37e74596c0b59e81b9ac58657f92297ef4ccb7ef/library/std/src/sys/pal/uefi/mod.rs#L36

@usamoi usamoi changed the title fix compilation on uefi or i686 fix compilation on uefi (with std feature enabled) or i686 Dec 18, 2024
@newpavlov
Copy link
Member

But we do not use RawOsError anywhere. Yes, on UEFI it's not i32 which we return from raw_os_error, but this method will always return None on UEFI targets with the existing backends. Even with a custom backend you will not be able to craft an Error from an UEFI code.

@newpavlov
Copy link
Member

Could you please split the Error changes into a separate PR? I can merge the other changes right away.

@usamoi
Copy link
Contributor Author

usamoi commented Dec 18, 2024

But we do not use RawOsError anywhere. Yes, on UEFI it's not i32 which we return from raw_os_error, but this method will always return None on UEFI targets with the existing backends. Even with a custom backend you will not be able to craft an Error from an UEFI code.

Ok. I'll remove code about Error.

@newpavlov
Copy link
Member

newpavlov commented Dec 18, 2024

It may be worth to reconsider our use of NonZeroU32 for error codes. IIRC previously we had a similar issue with Windows UWP backend which was using the higher bits similarly to the UEFI codes. And in future we may add a proper UEFI backend on top of EFI_RNG_PROTOCOL.

But it's unclear how it's better to do this. I don't think we should just double the Error size.

@newpavlov
Copy link
Member

Thank you!

@newpavlov newpavlov merged commit 9195c09 into rust-random:master Dec 18, 2024
61 checks passed
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

Successfully merging this pull request may close these issues.

2 participants