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: Fix unknown mt_srand() compatibility for unknown modes #13544

Merged
merged 1 commit into from Feb 29, 2024

Conversation

TimWolla
Copy link
Member

@TimWolla TimWolla commented Feb 27, 2024

see: https://3v4l.org/fOZbQ. Will need some adjustments when merging to PHP 8.3, due to the deprecation.

Found, because I wondered about the implicit downcast from zend_long to uint8_t during the assignment of mode and wondered about that, because I contemplated making the two mode constants and enum in PHP 8.4+.


PHP 8.1 and below interpreted unknown modes as MT_RAND_MT19937, but PHP 8.2+ interprets them as MT_RAND_PHP.

Align the behavior with PHP 8.1 and below, because folks should be steered towards the standard mode.

PHP 8.1 and below interpreted unknown modes as `MT_RAND_MT19937`, but PHP 8.2+
interprets them as `MT_RAND_PHP`.

Align the behavior with PHP 8.1 and below, because folks should be steered
towards the standard mode.
@TimWolla
Copy link
Member Author

Duh. Forgot to git add the test, now added.

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.

LGTM, might make sense to make this a ValueError in master.

@TimWolla
Copy link
Member Author

LGTM, might make sense to make this a ValueError in master.

Mt19937::__construct() already validates this and the days of the $mode parameter are counted anyway. So I'd rather not introduce additional breakage for folks that are still upgrading.

@TimWolla TimWolla merged commit e059498 into php:PHP-8.2 Feb 29, 2024
8 checks passed
@TimWolla TimWolla deleted the random-mt-srand-compat branch February 29, 2024 17:07
TimWolla added a commit that referenced this pull request Feb 29, 2024
* PHP-8.2:
  random: Fix unknown `mt_srand()` compatibility for unknown modes (#13544)
  Removed `REPORT_EXIT_STATUS=no` in libmysql tests
  Revert "Fix GH-13519: PGSQL_CONNECT_FORCE_RENEW with persistent connections." (#13546)
TimWolla added a commit that referenced this pull request Feb 29, 2024
* PHP-8.3:
  random: Fix unknown `mt_srand()` compatibility for unknown modes (#13544)
  Merge branch 'PHP-8.2' into PHP-8.3
  Removed `REPORT_EXIT_STATUS=no` in libmysql tests
  Revert "Fix GH-13519: PGSQL_CONNECT_FORCE_RENEW with persistent connections." (#13546)
TimWolla added a commit that referenced this pull request Feb 29, 2024
Apparently PHP 8.2.17 was branched off after creating the PR and before merging
it, placing the NEWS in the wrong location.
TimWolla added a commit that referenced this pull request Feb 29, 2024
* PHP-8.2:
  [ci skip] Fix version for GH-13544 in NEWS
TimWolla added a commit that referenced this pull request Feb 29, 2024
* PHP-8.3:
  [ci skip] Fix version for GH-13544 in NEWS
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

3 participants