-
Notifications
You must be signed in to change notification settings - Fork 190
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
Freebsd: Try getrandom() first #57
Conversation
Looks good, works fine. |
Maybe it's worth to make this PR independent from #54? I think it will be some time before we will converge our opinions on that one, while changes in this PR look quite straightforward. |
The main reason for the change is that this implementation uses the |
I do not insist on it, you can leave it as-is. I was simply thinking it may be easier for you, since no syncing with #54 would be needed. |
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.
Looks good, other than a comment as mentioned.
while !buf.is_empty() { | ||
let res = f(buf); | ||
if res < 0 { | ||
let err = io::Error::last_os_error(); |
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.
Seems odd to me to assume that calling last_os_error
is appropriate here given the function name. At least this should be mentioned by function documentation.
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.
During removal of std
dependency it will have to be replaced with an explicit errno
, so I think it's fine to keep it as-is for the time being.
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.
So I changed the function to be named sys_fill_exact
and added a comment to explain the expected semantics of the sys_fill
function. @newpavlov is right that this will be replaced with an explicit call to an errno function in #54
Depends on #54 fixes #35
Implementation is quite similar to the Solaris/Illumos implementation. @myfreeweb PTAL
Edit: Also adds a
fill_exact
helper method for making repeated OS calls to fill a buffer. This let's us simplify the Linux/Solaris/FreeBSD code, as well as adding some changes useful for #54 and #58