Repository navigation
bpo-40286: Makes simpler the relation between randbytes() and getrandbits() - #19574
serhiy-storchaka merged 1 commit into
Conversation
Sorry, something went wrong.
SystemRandom.random(), SystemRandom.getrandbits() and random.Random.seed(bytes) all use big endian. I don't have a strong preference about the endian. I chose big endian since it's the "network endian" and so commonly used in the wild. |
Sorry, something went wrong.
pitrou
left a comment
There was a problem hiding this comment.
Can you post benchmark numbers?
Sorry, something went wrong.
| if (n < 4) { | ||
| memcpy(ptr, &word, n); | ||
| /* Drop least significant bits */ | ||
| memcpy(ptr, (uint8_t *)&word + (4 - n), n); |
There was a problem hiding this comment.
It looks like this will give different results on big-endian and little-endian platforms, no?
Sorry, something went wrong.
There was a problem hiding this comment.
word was already converted to little endian.
Sorry, something went wrong.
| /* Convert to little endian */ | ||
| word = _Py_bswap32(word); | ||
| #endif | ||
| if (n < 4) { |
There was a problem hiding this comment.
Just a matter of taste, but personally I would move this epilog out of the loop and change the loop condition to n >= 4.
Sorry, something went wrong.
There was a problem hiding this comment.
Then you would need to repeat the following 5 lines:
uint32_t word = genrand_uint32(self);
#if PY_BIG_ENDIAN
/* Convert to little endian */
word = _Py_bswap32(word);
#endif
Sorry, something went wrong.
|
SystemRandom is not deterministic, so we can use any endianess with it and change it without breaking compatibility. seed() is unrelated too. The little endian is used because getrandbits() uses little endian. With big endian you would need to fill the bytes object from the end and copy words to non-aligned addresses. More complex and slower code. |
Sorry, something went wrong.
|
FYI test_random passed with my randbytes() change on this PPC64 buildbot which is big endian: |
Sorry, something went wrong.
|
Another example of big endian buildbot, PPC64 Fedora 3.x: |
Sorry, something went wrong.
There is no consistent difference. |
Sorry, something went wrong.
* master: (1985 commits) bpo-40179: Fix translation of #elif in Argument Clinic (pythonGH-19364) bpo-35967: Skip test with `uname -p` on Android (pythonGH-19577) bpo-40257: Improve help for the typing module (pythonGH-19546) Fix two typos in multiprocessing (pythonGH-19571) bpo-40286: Use random.randbytes() in tests (pythonGH-19575) bpo-40286: Makes simpler the relation between randbytes() and getrandbits() (pythonGH-19574) bpo-39894: Route calls from pathlib.Path.samefile() to os.stat() via the path accessor (pythonGH-18836) bpo-39897: Remove needless `Path(self.parent)` call, which makes `is_mount()` misbehave in `Path` subclasses. (pythonGH-18839) bpo-40282: Allow random.getrandbits(0) (pythonGH-19539) bpo-40302: UTF-32 encoder SWAB4() macro use a|b rather than a+b (pythonGH-19572) bpo-40302: Replace PY_INT64_T with int64_t (pythonGH-19573) bpo-40286: Add randbytes() method to random.Random (pythonGH-19527) bpo-39901: Move `pathlib.Path.owner()` and `group()` implementations into the path accessor. (pythonGH-18844) bpo-40300: Allow empty logging.Formatter.default_msec_format. (pythonGH-19551) bpo-40302: Add pycore_byteswap.h header file (pythonGH-19552) bpo-40287: Fix SpooledTemporaryFile.seek() return value (pythonGH-19540) Minor modernization and readability improvement to the tokenizer example (pythonGH-19558) bpo-40294: Fix _asyncio when module is loaded/unloaded multiple times (pythonGH-19542) Fix parameter names in assertIn() docs (pythonGH-18829) bpo-39793: use the same domain on make_msgid tests (python#18698) ...
It may also make randbytes() faster on littleendian platforms.
https://bugs.python.org/issue40286