Skip to content

Remove NDEBUG constant - #5529

Closed
Girgias wants to merge 1 commit into
php:masterfrom
Girgias:ndebug-remove-constant
Closed

Girgias wants to merge 1 commit into
php:masterfrom
Girgias:ndebug-remove-constant

Conversation

@Girgias

@Girgias Girgias commented May 5, 2020

Copy link
Copy Markdown
Member

It was only used in our patched libmagic in a very convoluted way.

The PHP_DEBUG and ZEND_DEBUG constants are always defined and should be used instead.

I came across this while working on #5526

@nikic

nikic commented May 5, 2020

Copy link
Copy Markdown
Member

NDEBUG is a standard C constant, I don't think we should drop it. This will break code using assert() rather than ZEND_ASSERT for example.

@Girgias

Girgias commented May 5, 2020

Copy link
Copy Markdown
Member Author

NDEBUG is a standard C constant, I don't think we should drop it. This will break code using assert() rather than ZEND_ASSERT for example.

TIL, should I keep the libmagic change and drop the rest?

@Girgias
Girgias force-pushed the ndebug-remove-constant branch from 8f35a3f to f8be942 Compare May 5, 2020 17:34
@nikic

nikic commented May 6, 2020

Copy link
Copy Markdown
Member

I'd prefer leaving libmagic alone as well, because #ifndef NDEBUG around variables used only in assert() is also a standard pattern.

@Girgias

Girgias commented May 6, 2020

Copy link
Copy Markdown
Member Author

I'd prefer leaving libmagic alone as well, because #ifndef NDEBUG around variables used only in assert() is also a standard pattern.

ACK

@Girgias Girgias closed this May 6, 2020
@Girgias
Girgias deleted the ndebug-remove-constant branch May 6, 2020 08:45
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.

2 participants