Unify lazy atomics in entropy backends #804
Conversation
newpavlov
left a comment
newpavlov
left a comment
There was a problem hiding this comment.
I would prefer to keep the NonNull stuff.
tamird
force-pushed
the
cleanup-atomics
branch
2 times, most recently
from
7ad436c to
a52ff00
Compare
tamird
left a comment
tamird
left a comment
There was a problem hiding this comment.
I would prefer to keep the
NonNullstuff.
Done.
There was a problem hiding this comment.
I think it's worth to split the lazy module to lazy_bool and lazy_ptr modules. It would allow us to remove the #![allow(dead_code)] part.
I also think it's fine to name LazyNonNull simply as LazyPtr and write proper docs for the structs since you no longer use macro to generate them.
| Some(fptr) => { | ||
| let getrandom_fn = unsafe { transmute::<NonNull<c_void>, GetRandomFn>(fptr) }; | ||
| let dangling_ptr = NonNull::dangling().as_ptr(); | ||
| let getrandom_fn = unsafe { transmute::<*mut c_void, GetRandomFn>(fptr.as_ptr()) }; |
There was a problem hiding this comment.
Why are you casting from NonNull<c_void> to *mut c_void?
There was a problem hiding this comment.
That's not what is happening here. We are using as_ptr() to get the *mut void and casting it to GetRandomFn. This is better than casting the NonNull directly because it doesn't rely on NonNull being repr(transparent).
There was a problem hiding this comment.
By "casting" I meant the fptr.as_ptr() part.
This is better than casting the NonNull directly because it doesn't rely on NonNull being repr(transparent).
I don't think it's "better". repr(transparent) is a promise made by std which for NonNull was introduced in Rust 1.29. We can rely on this property no less than *mut c_void being castable to a function pointer. For example, see rust-lang/rust#49220.
There was a problem hiding this comment.
Hm, looking into it a bit more carefully it looks like it's not as clear cut as I would've liked. I asked about it here.
There was a problem hiding this comment.
Seems better to sidestep this potential problem by just casting the raw pointer. Yes?
There was a problem hiding this comment.
Yeah, it looks like this aspect is surprisingly (in a bad way) under-specified...
Replace ad-hoc atomic lazy caches with shared lazy helpers in the Linux/Android fallback, NetBSD, RDRAND, and RNDR backends. Add `LazyPtr` alongside `LazyUsize` and `LazyBool` so pointer and boolean caches use the same initialization contract. This reduces duplicated cache logic and keeps backend probing/fallback semantics aligned while preserving the existing retry-until-cached behavior.
tamird
force-pushed
the
cleanup-atomics
branch
from
650a3e6 to
2b471c5
Compare
2 participants
Replace ad-hoc atomic lazy caches with shared lazy helpers in the Linux/Android fallback, NetBSD, RDRAND, and RNDR backends. Add
LazyPtralongsideLazyUsizeandLazyBoolso pointer and boolean caches use the same initialization contract.This reduces duplicated cache logic and keeps backend probing/fallback semantics aligned while preserving the existing retry-until-cached behavior.