Conversation
|
Test failure looks legit. |
|
Ah, thanks I missed this. I'll have a look. |
|
The test failure appears to be caused by a limitation of the "FTP server". Then again, if I don't debug this, the test usually succeeds, because the sockets are non-blocking, so the second response may actually be read as separate response. I'm going to commit a hack which lets the server sleep between the two responses. |
|
This is supposed to fix https://bugs.php.net/80901 as well. |
nikic
left a comment
nikic
left a comment
There was a problem hiding this comment.
Can you explain why this fixes https://bugs.php.net/bug.php?id=80901 as well? That one is about null-termination of the buffer, and I don't really see related changes here.
|
Ah, you're right; this does not fix https://bugs.php.net/80901. |
First we need to properly clear the `inbuf`, what is an amendment to commit d2881ad[1]. Then we need to report `php_pollfd_for_ms()` failures right away; just setting `errno` does not really help, since at least in some cases it would have been overwritten before we actually could check it. We use `php_socket_strerror()` to get a proper error message, and define `ETIMEDOUT` to the proper value on Windows; otherwise we catch the definition in errno.h, which is not compatible with WinSock. The proper solution for this issue would likely be to include something like ext/sockets/windows_common.h. Finally, we ensure that we only report warnings using `inbuf`, if it is not empty. [1] <http://git.php.net/?p=php-src.git;a=commit;h=d2881adcbc9be60de7e7d45a3316b0e11b7eb1e8>.
Apparently, macOS's error string for ETIMEDOUT is "Operation timed out".
|
It seems the test failures are unrelated to this patch. Any objections to merge it? |
2 participants
First we need to properly clear the
inbuf, what is an amendment tocommit d2881ad[1].
Then we need to report
php_pollfd_for_ms()failures right away; justsetting
errnodoes not really help, since at least in some cases itwould have been overwritten before we actually could check it. We use
php_socket_strerror()to get a proper error message, and defineETIMEDOUTto the proper value on Windows; otherwise we catch thedefinition in errno.h, which is not compatible with WinSock. The
proper solution for this issue would likely be to include something
like ext/sockets/windows_common.h.
Finally, we ensure that we only report warnings using
inbuf, if it isnot empty.
[1] http://git.php.net/?p=php-src.git;a=commit;h=d2881adcbc9be60de7e7d45a3316b0e11b7eb1e8.