Skip to content
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 SecRandomCopyBytes on MacOS #322

Merged
merged 2 commits into from
Mar 24, 2018

Conversation

pitdicker
Copy link
Contributor

I am cleaning up some old branches, and this is a change from October...
Using the syscall interface if available seems always better than reading from /dev/random. I can't find any data on which version of OS X was the first to ship the interface, but it seems to be have been around for a long time. It is also the interface used by ring.

@dhardy
Copy link
Member

dhardy commented Mar 23, 2018

I know very little about Macs but at least Travis seems happy enough.

According to the API this has been available since 10.7 which apparently was released in July 2011, so I guess that's good enough (otherwise it could potentially try falling back to /dev/random or urandom — though with an extern fn it would probably just fail at link time).

@pitdicker
Copy link
Contributor Author

Ah, you did find the minimum version. I promise I did a thorough search, but that was half a year ago 😄. With 10.7 as minimum version we are good to go, as that is also the minimum version supported by Rust.

@dhardy dhardy mentioned this pull request Mar 23, 2018
33 tasks
@@ -186,7 +186,7 @@ impl ReadRng {
not(target_os = "freebsd"),
not(target_os = "fuchsia"),
not(target_os = "ios"),
not(target_os = "nacl"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is removed because nacl is dead? Seems reasonable to me, but it is not mentioned in the commit message.

Copy link
Contributor Author

@pitdicker pitdicker Mar 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NaCl was removed in #225, but this was my original PR dhardy#19. Basically Rust no longer supports PNaCl, the primary use case, and we don't know of any use of Rust with NaCl.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, my only point was that this is not mentioned in the commit message.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then you have a good point 😄. But the actual commit that removed nacl was two months ago (this is just one line that was forgotten), so not much to do here.

@pitdicker
Copy link
Contributor Author

Is this ok to be merged?

@dhardy dhardy merged commit efa3c3e into rust-random:master Mar 24, 2018
@pitdicker pitdicker deleted the secrandomcopybytes branch March 24, 2018 14:51
@pitdicker pitdicker mentioned this pull request Mar 25, 2018
pitdicker pushed a commit that referenced this pull request Apr 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants