Skip to content

Remove unused variable 'error'#12438

Merged
ndossche merged 1 commit into
php:masterfrom
ndossche:pcntl-1
Oct 15, 2023
Merged

Remove unused variable 'error'#12438
ndossche merged 1 commit into
php:masterfrom
ndossche:pcntl-1

Conversation

@ndossche

Copy link
Copy Markdown
Member

Will also get rid of the potential allocation happening in zend_is_callable_ex().

Will also get rid of the potential allocation happening in
zend_is_callable_ex().
@ndossche ndossche marked this pull request as ready for review October 14, 2023 20:56

@Girgias Girgias left a comment

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.

LGTM

Comment thread ext/pcntl/pcntl.c

if (!zend_is_callable_ex(handle, NULL, 0, NULL, NULL, &error)) {
if (!zend_is_callable_ex(handle, NULL, 0, NULL, NULL, NULL)) {
PCNTL_G(last_error) = EINVAL;

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.

Does it actually make sense to set the last error here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think so, EINVAL is for invalid arguments, and the argument is definitely not valid if this check fails.

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.

Well sure, but also this used to return false and a warning and not a TypeError, but eh I don't think it's super important.

@ndossche ndossche merged commit 5465cea into php:master Oct 15, 2023
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