-
Notifications
You must be signed in to change notification settings - Fork 239
Add OsRng to getrandom #751
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
base: master
Are you sure you want to change the base?
Changes from all commits
d4a480a
d2f6a1d
bdbe38d
26cb7e9
254be58
ff68f2a
8bdcacf
1e873b4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| //! rand_core adapter | ||
| use crate::Error; | ||
| use rand_core::{TryCryptoRng, TryRngCore}; | ||
|
|
||
| /// An RNG over the operating-system's random data source | ||
| /// | ||
| /// This is a zero-sized struct. It can be freely constructed with just `OsRng`. | ||
| /// | ||
| /// This struct is also available as [`rand::rngs::OsRng`] when using [rand]. | ||
| /// | ||
| /// # Usage example | ||
| /// | ||
| /// `OsRng` implements [`TryRngCore`]: | ||
| /// ``` | ||
| /// use getrandom::{rand_core::TryRngCore, OsRng}; | ||
| /// | ||
| /// let mut key = [0u8; 32]; | ||
| /// OsRng.try_fill_bytes(&mut key).unwrap(); | ||
| /// ``` | ||
| /// | ||
| /// Using it as an [`RngCore`] is possible using [`TryRngCore::unwrap_err`]: | ||
| /// ``` | ||
| /// use getrandom::rand_core::{TryRngCore, RngCore}; | ||
| /// use getrandom::OsRng; | ||
| /// | ||
| /// let mut rng = OsRng.unwrap_err(); | ||
| /// let random_u64 = rng.next_u64(); | ||
| /// ``` | ||
| /// | ||
| /// [rand]: https://crates.io/crates/rand | ||
| /// [`rand::rngs::OsRng`]: https://docs.rs/rand/latest/rand/rngs/struct.OsRng.html | ||
| /// [`RngCore`]: rand_core::RngCore | ||
| #[derive(Clone, Copy, Debug, Default)] | ||
| pub struct OsRng; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would prefer for it to be named as
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Makes some sense... some backends like RDRAND are not OS features. |
||
|
|
||
| impl TryRngCore for OsRng { | ||
| type Error = Error; | ||
|
|
||
| #[inline] | ||
| fn try_next_u32(&mut self) -> Result<u32, Error> { | ||
| crate::u32() | ||
| } | ||
|
|
||
| #[inline] | ||
| fn try_next_u64(&mut self) -> Result<u64, Error> { | ||
| crate::u64() | ||
| } | ||
|
|
||
| #[inline] | ||
| fn try_fill_bytes(&mut self, dest: &mut [u8]) -> Result<(), Error> { | ||
| crate::fill(dest) | ||
| } | ||
| } | ||
|
|
||
| impl TryCryptoRng for OsRng {} | ||
|
|
||
| #[cfg(test)] | ||
| mod test { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's better to define these tests in
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Motivation? Having tests close to the feature definition is slightly preferable in my opinion (where tests are not too extensive at least).
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My general preference is to keep public API tests in integration tests (i.e. in the tests folder) and/or doctest, while using test modules (i.e. unit tests) only for private API testing. Since these tests are quite small, I think they can be simply merged into the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If the motivation is to improve visibility of integration tests, then moving to I'm disinclined to push these into a doc-test since (a) it hides it, (b) I would normally assume that doc-tests are only for documentation purposes and (c) failure reporting for doc tests is worse (slower and worse messages) than for other types of test. So I don't understand why you prefer both integration tests and doc tests over a local
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's just a matter of not duplicating tests which are already covered by doctests. I am fine with keeping the example as-is and moving the |
||
| use super::*; | ||
|
|
||
| #[test] | ||
| fn test_os_rng() { | ||
| let x = OsRng.try_next_u64().unwrap(); | ||
| let y = OsRng.try_next_u64().unwrap(); | ||
| assert!(x != 0); | ||
| assert!(x != y); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_construction() { | ||
| assert!(OsRng.try_next_u64().unwrap() != 0); | ||
| } | ||
| } | ||
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.
How about naming the feature
os_rng(orsys_rngif the type will be renamed)?