Skip to content

Check ext/pcntl required functions with for loop #14302

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 2 commits into from
Jun 7, 2024

Conversation

petk
Copy link
Member

@petk petk commented May 22, 2024

This omits defining redundant HAVE_ symbols since these are used unconditionally in ext/pcntl.

  • HAVE_FORK is defined via ext/standard/config.m4
  • HAVE_SIGACTION is defined via Zend.m4
  • HAVE_WAITPID symbol is removed

This omits defining redundant HAVE_<function> symbols since these are
used unconditionally in ext/pcntl.

* HAVE_FORK is defined via ext/standard/config.m4
* HAVE_SIGACTION is defined via Zend.m4
* HAVE_WAITPID symbol is removed
Copy link
Contributor

@morrisonlevi morrisonlevi left a comment

Choose a reason for hiding this comment

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

HAVE_WAITPID symbol is removed

As in, was previously unused so omitting is has no consequence?

@petk
Copy link
Member Author

petk commented May 23, 2024

HAVE_WAITPID symbol is removed

As in, was previously unused so omitting is has no consequence?

Indeed. HAVE_WAITPID is not used in php-src, and quick scan if some PHP extension might be using this symbol in a wrong way, also none.

@petk
Copy link
Member Author

petk commented Jun 4, 2024

Rechecked and all looks good here. I'll merge this in the following days. cc @devnexen

Copy link
Member

@devnexen devnexen left a comment

Choose a reason for hiding this comment

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

sorry I missed this one.

@petk petk merged commit cfb7395 into php:master Jun 7, 2024
2 checks passed
@petk petk deleted the patch-pcntl-loop branch June 7, 2024 20:49
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.

3 participants