Skip to content

random: Clean up seeding API #13540

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 9 commits into from
Feb 29, 2024
Merged

random: Clean up seeding API #13540

merged 9 commits into from
Feb 29, 2024

Conversation

TimWolla
Copy link
Member

As promised in #13350 (comment):

As a possible follow-up, I plan to remove the seed member from the struct php_random_algo and instead add well-typed engine specific seed functions, because seeding an engine generically does not appear super useful to me (Mt19937 ignores the upper half of the uint64_t seed for example).

@TimWolla
random: Expose xoshiro256**'s seeding functions
c92a3bb
@TimWolla
random: Expose pcgoneseq128xslrr64's seeding functions
04cbee6
@TimWolla
random: Expose Mt19937's seeding functions
c63d81f
@TimWolla
random: Expose CombinedLCG's seeding functions
305f880
@TimWolla
random: Call php_random_mt19937_seed32 to seed the global Mt19937
164692b 
This avoids the function pointer indirection and improves type safety.
@TimWolla
random: NULL the generic seeding function
e365e70 
Different engines work quite differently, it is not useful to attempt to seed
them in a generic way using a 64 bit integer. As an example Mt19937 completely
ignores the upper 32 bits.
@TimWolla
random: Remove the seed member from php_random_algo
ad1fd44 
See the explanation in the previous commit for the reasoning. This member is
unused since the previous commit and was not consistently available even before
that (specifically for the Secure engine).
@TimWolla
UPGRADING.INTERNALS
cbcaa05
@TimWolla
random: Remove useless cast in php_mt_srand()
ed18365
@TimWolla TimWolla requested a review from nielsdos February 27, 2024 20:49
@TimWolla TimWolla requested a review from zeriyoshi as a code owner February 27, 2024 20:49
nielsdos
nielsdos approved these changes Feb 27, 2024
View reviewed changes
Copy link
Member

@nielsdos nielsdos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good to me.
The macOS failure and Linux NTS failure are unrelated to this PR.

@TimWolla TimWolla merged commit 99e7cf0 into php:master Feb 29, 2024
@TimWolla TimWolla deleted the random-seed-api branch May 23, 2024 19:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nielsdos nielsdos nielsdos approved these changes

@Girgias Girgias Girgias approved these changes

@zeriyoshi zeriyoshi zeriyoshi approved these changes

Assignees
No one assigned
Labels
Extension: random
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@TimWolla @nielsdos @Girgias @zeriyoshi