Skip to content

Conversation

@dhardy
Copy link
Member

@dhardy dhardy commented Nov 8, 2025

The MSRV bump is required if we want to make getrandom depend on rand_core (see rust-random/rand#1675). Unless we use a split MSRV depending on whether rand_core is a dependency.

It is however a significant bump, so we might prefer not to.

@dhardy dhardy requested review from josephlr and newpavlov November 8, 2025 10:22
@newpavlov
Copy link
Member

newpavlov commented Nov 8, 2025

I am in favor of bumping MSRV and releasing v0.4 (see #722) regardless of whether OsRng will be part of getrandom or not.

(I will wait for CI to be fixed before approving)

matrix:
os: [ubuntu-24.04, windows-2022]
toolchain: [nightly, beta, stable, "1.63"]
toolchain: [nightly, beta, stable, "1.85"]
Copy link
Member

Choose a reason for hiding this comment

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

Ideally, we should extract MSRV from the rust-version field instead of manually providing it.

@dhardy
Copy link
Member Author

dhardy commented Nov 9, 2025

Releasing v0.4 works for me. Okay, I'll try to get this fixed.

@dhardy
Copy link
Member Author

dhardy commented Nov 9, 2025

So yesterday CI showed this error:

error[E0133]: call to unsafe function `core::arch::x86_64::_rdrand64_step` is unsafe and requires unsafe block
  --> src/backends/rdrand.rs:34:12
   |
34 |         if rdrand_step(&mut val) == 1 {
   |            ^^^^^^^^^^^^^^^^^^^^^ call to unsafe function

Today after adding unsafe, the RDRAND UEFI job fails:

error: unnecessary `unsafe` block
  --> src/backends/rdrand.rs:34:12
   |
34 |         if unsafe { rdrand_step(&mut val) } == 1 {
   |            ^^^^^^ unnecessary `unsafe` block
   |
   = note: `-D unused-unsafe` implied by `-D warnings`
   = help: to override `-D warnings` add `#[allow(unused_unsafe)]`

This is using nightly rustc. So why does it not think unsafe is necessary here?

@dhardy
Copy link
Member Author

dhardy commented Nov 9, 2025

Note: core::error::Error is stable since 1.81 so we may be able to drop the std feature after this. (New PR.)

Comment on lines 30 to 38
#[target_feature(enable = "rdrand")]
#[cfg_attr(target_os = "uefi", allow(unused_unsafe))] // HACK: Rust lint gives false positive on uefi
unsafe fn rdrand() -> Option<Word> {
for _ in 0..RETRY_LIMIT {
let mut val = 0;
if rdrand_step(&mut val) == 1 {
if unsafe { rdrand_step(&mut val) } == 1 {
return Some(val);
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

Got a better idea?

At least allow(unused_unsafe) is fairly harmless.

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