Update READMEs. by dhardy · Pull Request #328 · rust-random/rand

dhardy · 2018-03-23T15:38:33Z

Better doc of features in both crates.
Update version req to 0.5.0-pre.0. More updates are needed for final 0.5.

pitdicker

Looks like a good improvement to me.

Still, it it said that having an example in the readme is good practise.

pitdicker · 2018-03-23T19:07:27Z

rand_core/README.md

+comprising `RngCore` support for `Box<R>` types where `R: RngCore`, as well as
+extensions to the `Error` type's functionality.
+
+Due to a bug in Cargo, `rand_core` is built without `std` support by default.


This sounds a bit negative. Can you say "Due to {issue-number}"?

It was intended to be, and it's documented in Cargo.toml.

Maybe some day Cargo will have a lot less rough edges. I guess a flexible build/packaging tool turned out to be a considerably more complex than was initially envisaged. For now it still feels like a quick hack in many ways.

dhardy · 2018-03-24T12:15:11Z

Added another commit with a lot of new library level documentation.

I'm considering moving or removing the big examples at the end of the library documentation; they're good but it's just too much doc in one place. Thoughts?

pitdicker

Very nice improvements!

I'm considering moving or removing the big examples at the end of the library documentation; they're good but it's just too much doc in one place.

One of my wishes 😄.
It is nice documentation, but just too much in the wrong place. Is it possible to move it to a separate file?

pitdicker · 2018-03-24T12:23:00Z

src/lib.rs

+//! much smaller state and faster initialisation while its disadvantage is
+//! that it may be easy for an attacker to predict; actual performance varies).
+//! These can be seeded from a parent generator (`SeedableRng::thread_rng`) or
+//! with fresh entropy using `NewRng`:


pitdicker · 2018-03-24T12:28:28Z

src/lib.rs

+//! `thread_rng` and potentially much slower if `EntropyRng` must fall back to
+//! `JitterRng` as a source.
+//! 
+//! It is also common to use an algorithmic generator in local memory; for


I think it is better to have this one as the second paragraph, and to reword a bit.

It is also quite common, and can have advantages, to use an algorithmic generator in local memory.
StdRng and SmallRng provide to generators selected for common use. StdRng can be recommended for all uses, the main advantages of SmallRng are much smaller state and faster initialisation while its disadvantage is that it may be easy for an attacker to predict; actual performance varies.
...

I don't want to make the doc too long however — I'll try to find a compromise.

pitdicker · 2018-03-24T12:29:02Z

src/lib.rs

-//!
+//! 
+//! In case one wants specifically to have a reproducible stream of "random"
+//! data (e.g. to procedurally generate a game world), select a named algorithm


Do you want to link to the prng module here?

pitdicker · 2018-03-24T12:30:29Z

src/lib.rs

+//! data (e.g. to procedurally generate a game world), select a named algorithm
+//! (i.e. not `StdRng`/`SmallRng` which may be adjusted in the future), and
+//! use `SeedableRng::from_seed`. Some generators provide additional
+//! constructors suitable for some applications, e.g. `IsaacRng::new_from_u64`.


Mostly unrelated though: most RNGs now have a new_unseeded method. They are a thorn in my eye 😄. Should we deprecate them with 0.5?

I think that was originally the plan but got forgotten? Sure, why not.

pitdicker · 2018-03-24T12:44:06Z

src/lib.rs

+//! 
+//! The `seq` module has a few tools applicable to sliceable or iterable data.
+//! 
 //! # Cryptographic security


Your new paragraphs are a good improvement, but I don't like it completely yet. It reads as very serious (can be a good thing), but Rand is not really a library for cryptography. I would mostly point away the responsibility to specialized crates, when one wants to start using an RNG for encrypting and stuff.

What I would like to see somewhere (just noting from my lazy chair) is that for many algorithms and situations it is good to be unpredictable. That is what I see as the reason for including CSPRNGs. We should make it that they are not only a good idea for some corner of problems, but are a good choice for many situations that are seemingly security-unrelated. Most things that are somehow using an RNG in a situation that can be influenced by users, do well using one. One example is the standard library's HashMap, another something like random-pivot quicksort from the PCG blog.

Rand is not intended to be used for encryption directly (although there isn't much reason various generators couldn't be used as stream ciphers; just initialisation isn't standard), but I think Rand should be usable as a source of secrets — with caveats that (for now at least) the code hasn't had proper expert review and that currently we have limited means to mitigate side-channel attacks (which may or may not be improved upon).

BTW while this isn't intent to be a guide to cryptography I feel there are so many myths surrounding cryptography that it is worth trying to put people straight (e.g. distrust of CSPRNGs; the bigger problem is usually user-space PRNGs or poorly seeded PRNGs).

pitdicker · 2018-03-24T12:48:52Z

src/os.rs

+///     `/dev/random` is not better than `/dev/urandom` in most cases.  However,
+///     this means that `/dev/urandom` can yield somewhat predictable randomness
+///     if the entropy pool is very small, such as immediately after first
+///     booting.  Linux 3.17 added the `getrandom(2)` system call which solves


Maybe we should say a little less here, I am not sure this description is entirely up-to-date. Maybe we should just say that that the quality difference between /dev/random and /dev/urandom is a myth or at least outdated information. Making this a very short paragraph, as an addition to the list of system interfaces used.

It's also about getrandom; I did consider just referring to the man page. It's just a cut-and-paste.

Better doc of features in both crates. Update version req to 0.5.0-pre.0. More updates are needed for final 0.5.

dhardy · 2018-03-24T16:37:39Z

Updated (rebased + 1 new commit).

I'm not sure what to do with the doc on OsRng.

Edit: still need to link stuff on that lib doc.

Note: links are predictive. Unfortunately example code is not included in generated documentation, so we must link to the repo.

This uses relative paths between crates which works fine with locally-generated documentation and should also with this: https://rust-lang-nursery.github.io/rand/rand/index.html

dhardy · 2018-03-24T17:41:56Z

I extracted the examples and fixed links, plus a couple of trivial fixes.

pitdicker · 2018-03-24T18:28:55Z

Keeps getting better. Adding links was one of the things I wanted to suggest, but you did so already 👍. But I think you are not completely done with making changes, right?

dhardy · 2018-03-24T22:53:24Z

What makes you say that? I don't know what to do with the OsRng comments.

pitdicker · 2018-03-25T07:44:02Z

I did not mean to say that you should change any more things, more that that was the impression I got.

If it is up to me I'd say this is good to merge. Shall I try to take a stab at the documentation for OsRng?

To be honest I would like to see the change I suggested in #328 (comment). As a crate in the nursery, let's not sound to critical...

dhardy · 2018-03-25T15:19:58Z

No, I haven't any more changes. Go ahead and add your changes on top of this, or merge first if you like.

Update READMEs.

pitdicker reviewed Mar 23, 2018

View reviewed changes

pitdicker reviewed Mar 24, 2018

View reviewed changes

dhardy added 3 commits March 24, 2018 16:36

Update READMEs.

8ac8ac8

Better doc of features in both crates. Update version req to 0.5.0-pre.0. More updates are needed for final 0.5.

Doc: big update to library doc

d5d5ba6

Lib doc: update in response to review

eccc5c9

dhardy force-pushed the readme branch from e3d8515 to eccc5c9 Compare March 24, 2018 16:37

dhardy added 4 commits March 24, 2018 16:57

Travis: build doc for all sub-crates

57905a3

Extract examples from lib doc

8b65ebe

Note: links are predictive. Unfortunately example code is not included in generated documentation, so we must link to the repo.

Doc: fix lots of links

be98c64

This uses relative paths between crates which works fine with locally-generated documentation and should also with this: https://rust-lang-nursery.github.io/rand/rand/index.html

Readme: link both generated doc sites

4650ed8

Disable examples on no-std

5271ffb

pitdicker mentioned this pull request Mar 25, 2018

Changes to rand_core readme #330

Merged

pitdicker mentioned this pull request Mar 25, 2018

Osrng doc #333

Merged

pitdicker merged commit b264f38 into rust-random:master Mar 25, 2018

dhardy deleted the readme branch March 26, 2018 11:24

pitdicker added a commit that referenced this pull request Apr 4, 2018

Merge pull request #328 from dhardy/readme

5b6fe57

Update READMEs.

Uh oh!

Conversation

dhardy commented Mar 23, 2018

Uh oh!

pitdicker left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhardy commented Mar 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pitdicker left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dhardy commented Mar 24, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dhardy commented Mar 24, 2018

Uh oh!

pitdicker commented Mar 24, 2018

Uh oh!

dhardy commented Mar 24, 2018

Uh oh!

pitdicker commented Mar 25, 2018

Uh oh!

dhardy commented Mar 25, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dhardy commented Mar 24, 2018 •

edited

Loading

dhardy commented Mar 24, 2018 •

edited

Loading