improvements to random seed in src/tests.c #516
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
in case this code should ever be used as an example, a warning is a nice way of helping ensure insecure keys are not generated
the two middle arguments to fread() are easily confused, and cause the checking of return value to fail incorrectly (and possibly succeed incorrectly.)
|
utACK 8b3841c |
|
bump |
jonasnick
reviewed
Apr 11, 2018
src/tests.c
Outdated
| @@ -4918,7 +4918,8 @@ int main(int argc, char **argv) { | |||
| } | |||
| } else { | |||
| FILE *frand = fopen("/dev/urandom", "r"); | |||
| if ((frand == NULL) || fread(&seed16, sizeof(seed16), 1, frand) != sizeof(seed16)) { | |||
| if ((frand == NULL) || fread(&seed16, 1, sizeof(seed16), frand) != sizeof(seed16)) { | |||
| fprintf(stderr, "WARNING: could not read 16 bytes from /dev/urandom; falling back to insecure PRNG\n"); | |||
There was a problem hiding this comment.
src/tests.c: In function ‘main’:
src/tests.c:4923:13: warning: ISO C90 forbids mixed declarations and code [-Wdeclaration-after-statement]
uint64_t t = time(NULL) * (uint64_t)1337;
There was a problem hiding this comment.
@hdon to fix this, move the fprint down a line. Then we should be good to merge.
Reported-by: Jonas Nick <jonasd.nick@gmail.com>
|
@sipa @jonasnick @real-or-random @hdon please at least utACK my trivial fixup commit so I can merge. :P |
|
utACK |
gmaxwell
added a commit
that referenced
this pull request
Feb 21, 2019
be40c4d Fixup for C90 mixed declarations. (Gregory Maxwell) 8b3841c fix bug in fread() failure check (Don Viszneki) cddef0c tests: add warning message when /dev/urandom fails (Don Viszneki) Pull request description: I've made two small changes to `src/tests.c` circa random seed generation. Added a warning when `/dev/urandom` fails, mostly to defend against the case that someone should use the code verbatim, but also to enhance its illustrative power. Also I fixed a bug with how the return value of `fread()` was being evaluated. In fact, `/dev/urandom` was never being applied before as the check on the return value of `fread()` always failed! Tree-SHA512: 239dbe8316220c2f0e5b370bf9a18f78196e96cc4a7edea58cf2521b2c9cbc8da065be96aa859f90324d57e388d30f7670ce6bc1cca52e5162e5ca66b1a55b34
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've made two small changes to
src/tests.ccirca random seed generation.Added a warning when
/dev/urandomfails, mostly to defend against the case that someone should use the code verbatim, but also to enhance its illustrative power.Also I fixed a bug with how the return value of
fread()was being evaluated. In fact,/dev/urandomwas never being applied before as the check on the return value offread()always failed!