Skip to content

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

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

Merged
merged 1 commit into from
Aug 24, 2019

Conversation

Girgias
Copy link
Member

@Girgias Girgias commented Aug 23, 2019

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
Member

nikic commented Aug 23, 2019

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
Member Author

Girgias commented Aug 23, 2019

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.

@Girgias Girgias force-pushed the bogus-check-str-pad branch from cf2adfd to 49059c2 Compare August 24, 2019 13:57
@Girgias Girgias force-pushed the bogus-check-str-pad branch from 49059c2 to 9d18f23 Compare August 24, 2019 14:05
@php-pulls php-pulls merged commit 9d18f23 into php:master Aug 24, 2019
@Girgias Girgias deleted the bogus-check-str-pad branch August 24, 2019 16:08
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.

5 participants