bpo-40286: Makes simpler the relation between randbytes() and getrandbits() #19574
Conversation
serhiy-storchaka
force-pushed
the
random-randbytes-getrandbits
branch
from
ca69ca5
to
210abfe
Compare
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. |
| word = _Py_bswap32(word); | ||
| #endif | ||
| 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?
There was a problem hiding this comment.
word was already converted to little endian.
| #if PY_LITTLE_ENDIAN | ||
| /* Convert to big endian */ | ||
| #if PY_BIG_ENDIAN | ||
| /* 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.
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
|
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 |
|
FYI test_random passed with my randbytes() change on this PPC64 buildbot which is big endian: |
|
Another example of big endian buildbot, PPC64 Fedora 3.x: |
There is no consistent difference. |
* 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