Skip to content

Remove 'bogus' error condition in str_pad() - #4613

Merged
php-pulls merged 1 commit into
php:masterfrom
Girgias:bogus-check-str-pad
Aug 24, 2019
Merged

Remove 'bogus' error condition in str_pad()#4613
php-pulls merged 1 commit into
php:masterfrom
Girgias:bogus-check-str-pad

Conversation

@Girgias

Copy link
Copy Markdown
Member

As per @nikic request in #4601 (comment)

I'm not entirely sure we should drop it but more fix it in a way that memory allocation doesn't fail. Or is the fatal error alright ? Because even with the current check a fatal error is possible (see variation5 test)

@nikic

Copy link
Copy Markdown
Member

Fatal error is correct here. You'll have to adjust the test expectation for debug builds though (you can make liberal use of %s, we're only interested in seeing some form of OOM error, not the specific message).

@Girgias

Copy link
Copy Markdown
MemberAuthor

Fatal error is correct here. You'll have to adjust the test expectation for debug builds though (you can make liberal use of %s, we're only interested in seeing some form of OOM error, not the specific message).

Same should be done for chunk_split then I imagine.

Comment threadext/standard/tests/strings/str_pad_variation1.phpt Outdated
@Girgias
Girgiasforce-pushed the bogus-check-str-pad branch from cf2adfd to 49059c2CompareAugust 24, 2019 13:57
@Girgias
Girgiasforce-pushed the bogus-check-str-pad branch from 49059c2 to 9d18f23CompareAugust 24, 2019 14:05
@php-pulls
php-pulls merged commit 9d18f23 into php:masterAug 24, 2019
@Girgias
Girgias deleted the bogus-check-str-pad branch August 24, 2019 16:08
Sign up for freeto 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.

5 participants

@Girgias@nikic@KalleZ@php-pulls@kocsismate