Re: [RFC] [Under Discussion] Random Extension 5.x

Hi On 3/9/22 11:48, Go Kudo wrote:

Proposed RFCs and implementations have been reorganized. The main changes are as follows https://wiki.php.net/rfc/rng_extension https://github.com/php/php-src/pull/8094

I've compared the RFC against the implementation once more: - The RFC should clarify that returning an empty string in Engine::generate() is not allowed. - The corresponding test 'ext/random/tests/03_randomizer/user_unsafe.phpt' should verify not just an exception is thrown, but also the exception message (I'll add a review comment). - PCG64 effectively has 256 Bits of internal state (128 Bits for 's' and 128 Bits for 'inc'), but it only accepts 128 Bits for 's'. 'inc' cannot be provided by the user. The purpose of 'inc' is not entirely clear to me, but the user likely should be able to specify it for full reproducibility. - The default initialization of PCG64 with a null seed will fill 'inc', not 's'. This likely is a bug? At least it's inconsistent with the previous point. - The RFC is missing an explanation of the guarantees the implementation will (or will not) make. This is important for the user if they are relying on reproducibility of the sequences and outputs. The more guarantees we give, the less can be changed in the future. I've previously mentioned that in the thread for '4.x': https://externals.io/message/117026#117061 Best regards Tim Düsterhus