Skip to content

ext/standard: Fix range() calculated size in HT_MAX_SIZE ValueError#22513

Open
arshidkv12 wants to merge 2 commits into
php:PHP-8.4from
arshidkv12:range-array-1
Open

ext/standard: Fix range() calculated size in HT_MAX_SIZE ValueError#22513
arshidkv12 wants to merge 2 commits into
php:PHP-8.4from
arshidkv12:range-array-1

Conversation

@arshidkv12

Copy link
Copy Markdown
Contributor

…essage

ext/standard: Fix range() calculated size in HT_MAX_SIZE ValueError message

ext/standard: Fix range() calculated size in HT_MAX_SIZE ValueError message

ext/standard: Fix range() calculated size in HT_MAX_SIZE ValueError message
--TEST--
GH-22505 : range() reports the correct calculated size when exceeding HT_MAX_SIZE
--INI--
memory_limit=20G

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

is this test actually run on CI ??

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think, it is running. range(0, (1 << 30)) code needed minimum 17G.

Comment thread ext/standard/array.c Outdated
"The supplied range exceeds the maximum array size: start=" ZEND_LONG_FMT " end=" ZEND_LONG_FMT " step=" ZEND_LONG_FMT, end, start, (_step)); \
"The supplied range exceeds the maximum array size by %" PRIu64 " elements: " \
"start=" ZEND_LONG_FMT ", end=" ZEND_LONG_FMT ", step=" ZEND_LONG_FMT ". " \
"Calculated size: %" PRIu64 ". Maximum size: %" PRIu64 ".", \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I changed the error message to match the master branch. I think maintenance is easier now.

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.

2 participants