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
from
82c39d9 to
7ad436c
Compare
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
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.
| const NOT_AVAILABLE: NonNull<c_void> = unsafe { NonNull::new_unchecked(usize::MAX as *mut c_void) }; | ||
|
|
||
| static GETRANDOM_FN: AtomicPtr<c_void> = AtomicPtr::new(core::ptr::null_mut()); | ||
| const NOT_AVAILABLE: NonNull<c_void> = NonNull::dangling(); |
There was a problem hiding this comment.
I don't think this is correct. Quoting the method docs:
Note that the address of the returned pointer may potentially be that of a valid pointer, which means this must not be used as a “not yet initialized” sentinel value.
Meanwhile with the usize::MAX variant we can argue that it fundamentally can not be a valid function pointer.
| 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?
| } else { | ||
| // note: `transmute` is currently the only way to convert a pointer into a function reference | ||
| let getrandom_fn = unsafe { transmute::<NonNull<c_void>, GetRandomFn>(fptr) }; | ||
| let getrandom_fn = unsafe { transmute::<*mut c_void, GetRandomFn>(fptr.as_ptr()) }; |
| static GETRANDOM_FN: lazy::LazyNonNull<c_void> = lazy::LazyNonNull::new(); | ||
|
|
||
| let fptr = GETRANDOM_FN.unsync_init(init); | ||
| let fptr = unsafe { mem::transmute::<*mut c_void, GetRandomFn>(fptr.as_ptr()) }; |
|
|
||
| // Identical to LazyUsize except with bool instead of usize. | ||
| pub(crate) struct LazyBool(LazyUsize); | ||
| pub(crate) struct LazyBool(AtomicUsize); |
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.