-
Notifications
You must be signed in to change notification settings - Fork 182
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
Use configuration flags to switch between backends #504
Conversation
#[cfg(not(any(target_arch = "x86_64", target_arch = "x86")))] | ||
compile_error!("`rdrand` backend can be enabled only for x86 and x86-64 targets!"); | ||
#[path = "rdrand.rs"] mod imp; | ||
} else if #[cfg(getrandom_backend = "wasm_js")] { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could split wasm_js
feature further into wasm_node
and wasm_web
parts. But I don't know whether people need WASM files which can run both on Node and Web.
Alternatively, we could make the JS backend enabled by default and require users targeting non-JS WASM to enable the custom backend. But personally I dislike such approach since it's technically wrong and we play into the poor handling of the wasm32-unknown-unknown
target by the Rust ecosystem.
// `len`). | ||
let dest = uninit_slice_fill_zero(dest); | ||
let ret = unsafe { __getrandom_custom(dest.as_mut_ptr(), dest.len()) }; | ||
let ret = unsafe { __getrandom_custom(dest.as_mut_ptr().cast(), dest.len()) }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that in v0.3 we no longer need to zeroize the destination buffer since we can specify that uninitialized buffers may be passed to __getrandom_custom
.
We also probably could use NonZeroUsize
for len
to improve code generation for __getrandom_custom
provider.
#[path = "js.rs"] mod imp; | ||
} else if #[cfg(getrandom_backend = "esp_idf")] { | ||
#[cfg(not(target_os = "espidf"))] | ||
compile_error!("`esp_idf` backend can be enabled only for ESP-IDF targets!"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESP-IDF support is now optional because of the concerns raised in #397.
cc @briansmith |
This PR removes
linux_disable_fallback
,rdrand
,js
,test-in-browser
, andcustom
crate features. As their replacement two new configuration flags are introduced:getrandom_browser_test
andgetrandom_backend
. The latter can have the following values:custom
,rdrand
,linux_getrandom
,wasm_js
,esp_idf
.getrandom_backend
allows to change default backends which resolves issues like #346 and provides more flexibility to users. For example, it allows to use RDRAND or RDRND (see #494) directly instead of syscall-based interfaces. We previously did not allow such overwrites because of security concerns, but they do not apply in this case since configuration flags used by a project can not be controlled by its upstream dependencies.The
register_custom_getrandom!
macro is removed in favor of explicitly defining the__getrandom_custom
function. It does not look like the macro was widely used in practice and it's probably easier to explain theextern fn
approach (especially to embedded developers) than the "magical" registration macro.The new configuration flags also allow a great simplification of our testing code.
Finally, ESP-IDF support is no longer enabled by default because of the concerns raised in #397. Users can enable it by enabling the
esp_idf
opt-in backend.Closes #230
Closes #346
Closes #397
Closes #498