-
Notifications
You must be signed in to change notification settings - Fork 186
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
Remove libstd dependancy for Opening and Reading files #58
Conversation
116738a
to
1426f2e
Compare
As you can read in the linked ring issue, I made so such suggestion. |
Whoops sorry about that, meant to say you pointed out the libsodium implementation, changed to be more accurate. |
I guess it's time for a rebase — however I believe the only motivation is rust-lang/rust#62082, thus ideally this should wait on a decision whether we should go ahead with that (except that decision may depend on the existence of a viable PR...) |
@dhardy I rebased this PR, and the CI is passing. I agree that if we don't do rust-lang/rust#62082, we should not merge this PR. |
You also have to disable default features for |
Done |
As rust-lang/rust#62516 might take awhile to reslove. I just added in the RHEL 5 compat code. It only takes 2 lines. |
So I think we can merge this without waiting for decision regarding rust-lang/rust#62082, if |
I slightly changed the
Sounds reasonable to me. |
Can you also update the table in |
Done, I also fixed up the |
I think you can simplify the loop even further like this: let ret = loop {
// A negative timeout means an infinite timeout.
let res = unsafe { libc::poll(&mut pfd, 1, -1) };
if res == 1 {
break unsafe { open_readonly("/dev/urandom\0") };
} else if res < 0 {
let e = last_os_error().raw_os_error();
if e == Some(libc::EINTR) || e == Some(libc::EAGAIN) {
continue;
}
}
break None;
};
unsafe { libc::close(pfd.fd) };
ret |
Done. I totally forgot that rust loops can return a value. |
target_os = "solaris", | ||
))] | ||
// std-only trait definitions | ||
#[cfg(feature = "std")] | ||
mod error_impls; |
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.
What happens in this case when the getrandom
crate is used both by std
and by other crates wanting std
features?
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.
IIUC right now getrandom
will be compiled into std
, so enabling features for it during usual builds will not influence the copy of the crate used by std
. Although I am not sure how it will work with rust-lang/rfcs#2663.
There may not be another way to go about this, so fair enough. |
@newpavlov if we're going with the build script approach in #69, do we still want to wait on that PR before merging this one? |
No, I think we can merge it right away. |
Wasn't the plan to use this crate in libstd? Rust officially supports Linux 2.6.18+. |
Description was not updated to reflect #58 (comment) |
Depends on #54 so only look at the last commit if you want to just review this PR.
This PR removes the last part of
libstd
we were depending on (when not using thestd
feature). Specifically:libc::open(path.as_ptr(), libc::O_RDONLY | libc::O_CLOEXEC)
libc::O_CLOEXEC
(kernel < 2.6.22)/dev/random
we nowpoll(2)
on the/dev/random
file descriptor/dev/urandom
is safe/dev/random
/dev/urandom
has been seeded before using it. briansmith/ring#558