Skip to content

Remove unused ZEND_STACK_GROWS_DOWNWARDS constant#11762

Merged
arnaud-lb merged 1 commit intophp:masterfrom
petk:patch-autoconf-1
Jul 22, 2023
Merged

Remove unused ZEND_STACK_GROWS_DOWNWARDS constant#11762
arnaud-lb merged 1 commit intophp:masterfrom
petk:patch-autoconf-1

Conversation

@petk
Copy link
Member

@petk petk commented Jul 22, 2023 •
edited
Loading

  • Check for pthread_attr_getstack defines HAVE_PTHREAD_ATTR_GETSTACK which is not used in the code.
  • ZEND_STACK_GROWS_DOWNWARDS is also not used in the code.

@petk petk requested a review from iluuu1994 as a code owner July 22, 2023 10:06
devnexen
devnexen approved these changes Jul 22, 2023
View reviewed changes
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.

Looks good even tough I wonder where HAVE_PTHREAD_ATTR_GET_STACK comes from (typo maybe) as it does not apply for FreeBSD it is pthead_attr_getstack too. Could be fixed separately eventually.

devnexen
devnexen approved these changes Jul 22, 2023
View reviewed changes
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.

Looks good even tough I wonder where HAVE_PTHREAD_ATTR_GET_STACK comes from (typo maybe) as it does not apply for FreeBSD it is pthead_attr_getstack too. Could be fixed separately eventually.

@petk
Copy link
Member Author

petk commented Jul 22, 2023 •
edited
Loading

@devnexen, when running ./buildconf it creates main/php_config.h.in with this:

/* Define to 1 if you have the `pthread_attr_getstack' function. */
#undef HAVE_PTHREAD_ATTR_GETSTACK

then when running ./configure it runs the check for the pthread_attr_getstack. If pthreads.h is there, which probably is on some of the Linux and BSD variants, then this is added to the newly created main/php_config.h:

/* Define to 1 if you have the `pthread_attr_getstack' function. */
#define HAVE_PTHREAD_ATTR_GETSTACK 1

Perhaps it was meant for the Zend/zend_call_stack.c where pthread_get_stackaddr_np is used and guard with __APPLE__ and HAVE_PTHREAD_GET_STACKADDR_NP is added there. Yes, probably...

@petk petk mentioned this pull request Jul 22, 2023
Merged
@petk petk force-pushed the patch-autoconf-1 branch from 0a19a04 to 3a1d8ae Compare July 22, 2023 15:42
@petk petk changed the title Remove two unused constants Remove unused ZEND_STACK_GROWS_DOWNWARDS constant Jul 22, 2023
@petk
Copy link
Member Author

petk commented Jul 22, 2023

I've changed this into only a single change due to usage of pthread_attr_getstack and typo constant HAVE_PTHREAD_ATTR_GET_STACK which is being resolved here then #11766

@arnaud-lb arnaud-lb merged commit 9911ab7 into php:master Jul 22, 2023
@arnaud-lb
Copy link
Member

arnaud-lb commented Jul 22, 2023

Thank you!

@petk petk deleted the patch-autoconf-1 branch July 22, 2023 17:52
jorgsowa pushed a commit to jorgsowa/php-src that referenced this pull request Jul 26, 2023
@petk @jorgsowa
Remove unused ZEND_STACK_GROWS_DOWNWARDS constant (php#11762)
d812639
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devnexen devnexen devnexen approved these changes

@iluuu1994 iluuu1994 Awaiting requested review from iluuu1994

Assignees

No one assigned

Labels

Category: Build System Category: Engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@petk @arnaud-lb @devnexen