GH-10141: Fix typo #11624
GH-10141: Fix typo #11624
Conversation
In configure.ac check for function strerror_r defines constant HAVE_STRERROR_R.
On x86_64 glibc memrchr() uses SSE/AVX CPU extensions and works much faster then naive loop. On x86 32-bit we still use inlined version. memrchr() is a GNU extension. Its prototype becomes available when <string.h> is included with defined _GNU_SOURCE macro. Previously, we defined it in "php_config.h", but some sources may include <string.h> befire it. To avod mess we also pass -D_GNU_SOURCE to C compiler.
|
Thanks for catching this. The patch should be targeted to PHP-8.1 |
| @@ -1639,7 +1639,7 @@ ZEND_API ZEND_COLD ZEND_NORETURN void zend_error_noreturn(int type, const char * | |||
|
|
|||
| ZEND_API ZEND_COLD ZEND_NORETURN void zend_strerror_noreturn(int type, int errn, const char *message) | |||
| { | |||
| #ifdef HAVE_STR_ERROR_R | |||
| #ifdef HAVE_STRERROR_R | |||
| char buf[1024]; | |||
| strerror_r(errn, buf, sizeof(buf)); | |||
There was a problem hiding this comment.
@dstogov: I don't think this resolves the issue I mentioned in 067df26#commitcomment-121630134. The constant typo being fixed here was a separate issue that I overlooked entirely (great catch by @petk).
The GNU version of strerror_r(3) may return a pointer to an immutable string instead of storing the result in the buf parameter. The XSI version always sets buf and returns an integer to indicate success/error.
Again, I don't think this would result in a build error. The zend_error_noreturn() line below would just end up printing message and errn with nothing between since buf would remain empty.
There was a problem hiding this comment.
since buf would remain empty
Correction: buf would be uninitialized. I was incorrectly thinking of of static/global vars when I wrote that, which are zero-allocated.
|
Just a quick update here: I haven't been able to check this one out yet due various Can maybe @dunglas help us out a bit? Not related to this issue exactly, but there seemed to be a continuation planned here based on the comment in Or should the ZEND_MAX_EXECUTION_TIMERS be turned on by default in PHP-8.4? |
|
Thanks for catching my typo and working on fixing this problem. Regarding the continuation, is has been done in #10778. |
|
I fixed the issue in #11882. I wasn't aware of |
|
Perfect. Thank you @dunglas! I'll close this PR in favor of the above fix. |
In configure.ac check for function strerror_r defines constant HAVE_STRERROR_R.
The functionality was added in #10141 but I think that due to this change maybe PHP 8.3 version would be enough to fix it like this. Otherwise, in PHP 8.2 and 8.1 it will always do only this
char *buf = strerror(errn);.Edit: This needs to be adjusted a bit further due to unused result issue in the strerror_r() call.