Skip to content

Conversation

@asgrim
Copy link
Contributor

@asgrim asgrim commented Feb 16, 2016

See https://bugs.php.net/bug.php?id=71575

This removes extra semicolons outside macro calls when including php.h.

@nikic
Copy link
Member

nikic commented Feb 16, 2016

There's a few more occurrences of this particular semicolon, would be nice to fix all of them at once: http://lxr.php.net/search?q=ZEND_EXTERN_MODULE_GLOBALS&defs=&refs=&path=&hist=&project=PHP_MASTER

Zend/zend.h Outdated
#endif

ZEND_TSRMLS_CACHE_EXTERN();
ZEND_TSRMLS_CACHE_EXTERN()
Copy link
Member

Choose a reason for hiding this comment

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

This macro requires a semicolon under ZTS. If we want to change this, the semicolon needs to be added to the ZTS macro definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay; not sure I'd feel comfortable changing that just yet, so I've reverted this one.

@asgrim asgrim force-pushed the bugfix/71575-remove-extra-semicolons branch from 0b02476 to 24ed48e Compare February 16, 2016 23:17
@asgrim
Copy link
Contributor Author

asgrim commented Feb 16, 2016

@nikic thanks; I grepped and updated all the occurrences I could find.

@laruence
Copy link
Member

actually, these ; are no harm, and is good for auto-indent tools. why it bother you?

@laruence
Copy link
Member

oh, sorry, I didn't read the bug report. I get it now. thanks

@nikic
Copy link
Member

nikic commented Mar 3, 2016

Merged as c4b1888, also did the fix for the TSRM externs in 1ac1529.

@nikic nikic closed this Mar 3, 2016
@asgrim asgrim deleted the bugfix/71575-remove-extra-semicolons branch March 3, 2016 16:14
@asgrim
Copy link
Contributor Author

asgrim commented Mar 3, 2016

Thanks @nikic

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants