Skip to content

Conversation

usamoi
Copy link

@usamoi usamoi commented Oct 7, 2025

  • Added a CHANGELOG.md entry

Summary

On uefi targets, the return type of std::io::Error::raw_os_error is Option<usize>, instead of Option<i32>. In rand_core, it is assumed as Option<i32>. This PR uses Option<usize> as the return type on the uefi targets.

getrandom has already handled this: https://github.com/rust-random/getrandom/blob/18d89843981b93032b2a2c6f1e33897075a8d727/src/error.rs#L6-L19

Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't feel like the right way to do this: clearly #1537 failed to isolate rand from getrandom changes.

Do you need a patch release for rand_core?

@newpavlov
Copy link
Member

newpavlov commented Oct 7, 2025

Note that technically it's a breaking change. I think we can cfg the implementation for UEFI and replace it with panic and fix it properly in the next breaking release.

UPD: On the second thought, since the current code does not compile on the UEFI targets, then this change can be considered non-breaking since it only affects "broken" targets.

@usamoi
Copy link
Author

usamoi commented Oct 7, 2025

Do you need a patch release for rand_core?

No. I can use the patch locally, so this is not necessary for me.

Note that technically it's a breaking change.

I'm fine with waiting to merge it into 0.10.

@dhardy
Copy link
Member

dhardy commented Oct 15, 2025

@usamoi could you add a note to rand_core/CHANGELOG.md please?

@newpavlov I think RawOsError should be a pub type in getrandom and re-exported in rand_core to avoid this: it's a foot-gun that the docs just say raw_os_error returns Option<i32>. This is what e.g. libc does for platform-dependent types.

@newpavlov
Copy link
Member

@dhardy
I don't think it makes sense to both re-export RawOsError and wrap getrandom::Error. I think we should either expose getrandom::Error in rand_core (in the next breaking release) or duplicate RawOsError.

@dhardy
Copy link
Member

dhardy commented Oct 15, 2025

@newpavlov the argument not to export getrandom::Error was to minimise the chance that a breaking release of getrandom would be breaking in rand_core. So far that hasn't actually helped us though.

getrandom should make the RawOsError type explicit anyway IMO.

@dhardy
Copy link
Member

dhardy commented Oct 15, 2025

So there is a fundamental difference between a pub-export of getrandom::Error and getrandom::RawOsError: the former is a type, the latter an alias. In other words, getrandom_v0_3::Error and getrandom_v0_3::Error will be distinct types; getrandom_v0_3::RawOsError and getrandom_v0_3::RawOsError will not be.

Implication: depending on getrandom::Error automatically makes a bump to the getrandom (non-patch) version breaking; depending on getrandom::RawOsError does not (so long as the aliases are compatible).

Once getrandom v1.0 is out I'm fine with depending on getrandom::Error. See also #1537.

Copy link
Member

@dhardy dhardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Marking as 👍 but if the next getrandom release is made soon this may be replaced.

newpavlov pushed a commit to rust-random/getrandom that referenced this pull request Oct 15, 2025
@dhardy dhardy mentioned this pull request Oct 16, 2025
6 tasks
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.

3 participants