Skip to content
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.

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

random: Remove RAND_RANGE_BADSCALING #12374

Merged
merged 2 commits into from Oct 7, 2023

Conversation

TimWolla
Copy link
Member

@TimWolla TimWolla commented Oct 6, 2023

This macro is no longer used within php-src since
60ace13, it invokes undefined behavior depending on the input and the corresponding MT_RAND_PHP mode was deprecated in PHP 8.3.

Thus remove this macro. Any remaining non-php-src user should just inline it into their code, but should ideally migrate to a non-biased scaler. In any case the undefined behavior of the original implementation should be accounted for.

This macro is no longer used within php-src since
60ace13, it invokes undefined behavior
depending on the input and the corresponding MT_RAND_PHP mode was deprecated in
PHP 8.3.

Thus remove this macro. Any remaining non-php-src user should just inline it
into their code, but should ideally migrate to a non-biased scaler. In any case
the undefined behavior of the original implementation should be accounted for.
Copy link
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

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

UPGRADING.INTERNALS entry is probably needed.

@TimWolla
Copy link
Member Author

TimWolla commented Oct 6, 2023

UPGRADING.INTERNALS entry is probably needed.

Makes sense, done.

@Girgias Girgias merged commit 3bc63a3 into php:master Oct 7, 2023
3 checks passed
@TimWolla TimWolla deleted the rand-range-badscaling branch October 7, 2023 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants