noticed some incorrect error handling for some socket specific API's on windows
[email protected] (Sriram Natarajan)
| Newsgroups | php.internals.win |
|---|---|
| Message-ID | <[email protected]> |
HI On Windows , socket API's does not return '-1' on failure. In this case, most commonly returned error code is either SOCKET_ERROR or INVALID_SOCKET. Af course, most of us know this. However, I did notice that within our php implementation in some places we do check for the same BSD style error value (< 0) rather than using the appropriate macros. Here is a suggested patch to address this. Here, the issue is - if we didn't catch the error at the right place, we might end up providing incorrect error message(s). Please let me know, if I overlooked some thing obvious here. thanks sriram
socket-return-check.patch
(text/plain, 2.6 KB)
Index: ext/ftp/ftp.c
===================================================================
--- ext/ftp/ftp.c (revision 287965)
+++ ext/ftp/ftp.c (working copy)
@@ -147,7 +147,7 @@
size = sizeof(ftp->localaddr);
memset(&ftp->localaddr, 0, size);
- if (getsockname(ftp->fd, (struct sockaddr*) &ftp->localaddr, &size) == -1) {
+ if (getsockname(ftp->fd, (struct sockaddr*) &ftp->localaddr, &size) == SOCK_CONN_ERR) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "getsockname failed: %s (%d)", strerror(errno), errno);
goto bail;
}
@@ -1387,7 +1387,7 @@
sa = (struct sockaddr *) &ftp->localaddr;
/* bind/listen */
- if ((fd = socket(sa->sa_family, SOCK_STREAM, 0)) == -1) {
+ if ((fd = socket(sa->sa_family, SOCK_STREAM, 0)) == SOCK_CONN_ERR) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "socket() failed: %s (%d)", strerror(errno), errno);
goto bail;
}
@@ -1420,17 +1420,17 @@
php_any_addr(sa->sa_family, &addr, 0);
size = php_sockaddr_size(&addr);
- if (bind(fd, (struct sockaddr*) &addr, size) == -1) {
+ if (bind(fd, (struct sockaddr*) &addr, size) == SOCK_CONN_ERR) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "bind() failed: %s (%d)", strerror(errno), errno);
goto bail;
}
- if (getsockname(fd, (struct sockaddr*) &addr, &size) == -1) {
+ if (getsockname(fd, (struct sockaddr*) &addr, &size) == SOCK_CONN_ERR) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "getsockname() failed: %s (%d)", strerror(errno), errno);
goto bail;
}
- if (listen(fd, 5) == -1) {
+ if (listen(fd, 5) == SOCK_CONN_ERR) {
php_error_docref(NULL TSRMLS_CC, E_WARNING, "listen() failed: %s (%d)", strerror(errno), errno);
goto bail;
}
Index: main/network.c
===================================================================
--- main/network.c (revision 287965)
+++ main/network.c (working copy)
@@ -308,7 +308,7 @@
SET_SOCKET_BLOCKING_MODE(sockfd, orig_flags);
- if ((n = connect(sockfd, addr, addrlen)) < 0) {
+ if ((n = connect(sockfd, addr, addrlen)) == SOCK_CONN_ERR) {
error = php_socket_errno();
if (error_code) {
@@ -342,7 +342,7 @@
BSD-derived systems set errno correctly
Solaris returns -1 from getsockopt in case of error
*/
- if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (char*)&error, &len) < 0) {
+ if (getsockopt(sockfd, SOL_SOCKET, SO_ERROR, (char*)&error, &len) == SOCK_CONN_ERR) {
ret = -1;
}
} else {
@@ -709,7 +709,7 @@
clisock = accept(srvsock, (struct sockaddr*)&sa, &sl);
- if (clisock >= 0) {
+ if (clisock != SOCK_CONN_ERR) {
php_network_populate_name_from_sockaddr((struct sockaddr*)&sa, sl,
textaddr, textaddrlen,
addr, addrlen