-
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
Conditionally disable file fallback for Android and Linux #396
Conversation
target_arch
ese1ffdf6
to
2cc9413
Compare
// getrandom(2) was introduced in Linux 3.17 | ||
static HAS_GETRANDOM: LazyBool = LazyBool::new(); | ||
if HAS_GETRANDOM.unsync_init(is_getrandom_available) { | ||
sys_fill_exact(dest, getrandom_syscall) |
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.
Sorry to contradict my earlier suggestion, but I think I have a better one: Let's change this line to linux_android::getrandom_inner(dest)
. This way, it will crystal clear that the logic is EXACTLY the same as the no-fallback case. This would also allow you to keep getrandom_syscall in linux_android, which I think would be better than putting it in libc_util.
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 need getrandom_syscall
for checking getrandom
availability, so I think the current solution is fine.
9c60413
to
7a4ed41
Compare
@josephlr WDYT? |
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.
This looks good! Thanks for working on this.
My only concerns are with how the new Cargo feature should work. If you want, you can merge the rest of this and we can discuss in a followup PR (or we can discuss it here).
I think we can discuss it here. It looks like the Nightly is broken on some targets which causes breakage of some CI jobs, so I do not plan to merge it right away. |
The CI failures are caused by Nightly issues and not relevant to this PR, so we can merge it. |
The file fallback is left enabled for Android targets and for Linux targets which support pre-3.17 kernels.
This PR also adds
linux_disable_fallback
crate feature and removes catching ofEPERM
duringgetrandom
availability check on Android targets.Closes #376
Closes #229
cc @briansmith