Repository navigation
Sync #if/ifdef/defined (-Wundef) - #14623
Conversation
These are either define (to value 1) or undefined: - __GNUC__ - DBA_CDB_BUILTIN - DBA_GDBM - HAVE_FORK - HAVE_PUTENV - HAVE_SETENV - HAVE_SYS_SELECT_H - HAVE_SYS_SOCKET_H - HAVE_SYS_WAIT_H - HAVE_UNSETENV - RFC3678_API - ZEND_ENABLE_ZVAL_LONG64 - ZTS Follow-up of phpGH-5526
Girgias
left a comment
There was a problem hiding this comment.
Not sure about the sockets/multicast.c change and the ZendAccelerator one, but the rest looks good.
| void LSAPI_Log(int flag, const char * fmt, ...) | ||
| #if __GNUC__ | ||
| #ifdef __GNUC__ | ||
| __attribute__((format(printf, 2, 3))) | ||
| #endif |
There was a problem hiding this comment.
Shouldn't this just use ZEND_ATTRIBUTE_FORMAT ? or is this file not including anything from Zend/PHP currently?
There was a problem hiding this comment.
Then I think it can be added in the includes, yes. Ok, good one. Thanks for finding it.
There was a problem hiding this comment.
I've added the zend_portability.h in it and used ZEND_ATTRIBUTE_FORMAT.
| void LSAPI_Log(int flag, const char * fmt, ...) | ||
| #if __GNUC__ | ||
| __attribute__((format(printf, 2, 3))) | ||
| #endif | ||
| ; | ||
|
|
||
| void LSAPI_Log(int flag, const char * fmt, ...) ZEND_ATTRIBUTE_FORMAT(printf, 2, 3); |
There was a problem hiding this comment.
This seems to be isolated from Zend, I think I prefer what you had before so we don't have unnecessary dependency on Zend if not needed.
There was a problem hiding this comment.
Sure, added back as it was with only ifdef changed.
I was already thinking about these __attribute__ items and how to improve it. It might fit a bit better together once we add a minimum compiler requirement in the future to rely on __has_attribute feature that comes with GCC 5 and later and clang 2.9 and later (I think). With some other adjustments for some almost obsolete platforms. Because now PHP doesn't have this resolved optimally yet.
The lsapilib.h shouldn't at this point depend on Zend.
|
Merge coming up then since these are all not problematic changes in terms of behavior. I've missed the comment above about the RFC3678_API in the multicast.c. From what I see, this one is defined in the PHP itself and not in the system header(s). It is also already used with #ifdef in the PHP-src code, so I think it's all good. |
These are either define (to value 1) or undefined:
__GNUC__Follow-up of GH-5526