random: add random_bytes() function#7390
Conversation
|
please provide a description of the purpose of this PR, and any other relevant information. |
sys/random/prng_tinymt32.c
Outdated
| uint32_t last; | ||
|
|
||
| while(size > sizeof(uint32_t)) { | ||
| *((uint32_t*)(buf+count)) = tinymt32_generate_uint32(&_random); |
There was a problem hiding this comment.
This code may (/will) cause alignment errors on ARM systems, as alignof(uint8_t) <= alignof(uint32_t).
There was a problem hiding this comment.
Thanks for the hint. Is there any proper way to avoid this?
There was a problem hiding this comment.
One way would be to round the buffer pointer up to nearest 32 bit word and perform the loop. Finally generate two more words of random and use them to fill the leading and trailing bytes of the buffer.
There was a problem hiding this comment.
I believe this should be fixed in the newest commit
|
Needs rebase |
4ac5e53 to
bd33a21
Compare
|
@gebart done |
jnohlgard
left a comment
There was a problem hiding this comment.
This looks fine except for the stray extra include.
However, we are not testing this functionality in any of the existing tests, I suggest we create a follow-up PR after this PR and #7421 have been merged to update the implementation in #7421 to use random_bytes. That way, random_bytes will be tested implicitly by the unit tests in #7421.
Please remove the unused include, then squash
sys/include/random.h
Outdated
| #define RANDOM_H | ||
|
|
||
| #include <inttypes.h> | ||
| #include <stdlib.h> |
There was a problem hiding this comment.
This is not used within this header if I am not mistaken.
There was a problem hiding this comment.
absolutely right, not sure why I put it there
|
turns out the include probably wasn't as useless as we thought, I added |
jnohlgard
left a comment
There was a problem hiding this comment.
This looks fine now, ACK.
Let's implement the test in a separate PR
| @@ -27,6 +27,7 @@ | |||
| #define RANDOM_H | |||
|
|
|||
| #include <inttypes.h> | |||
There was a problem hiding this comment.
not really this PR's fault but this should be stdint.h instead. stdint provides the uintxx_t which I assume is the reason for this header. inttypes in addition provides the PRIx32 etc printf format macros as well as some more stuff which is not used by this API header. (You don't have to address this comment in this PR)
|
and go |
Adds a
random_bytes()function allowing to read a specified amount of bytes to a buffer. This might be useful, as currently it is only possible to read 32-bit integers from the random module.Possible use cases include #7393.