Re: [INTERNALS-WIN] noticed some incorrect error handling for some socket specific API's on windows

hi Sriram,

Looks good, please commit :)

Cheers,

On Wed, Sep 2, 2009 at 12:16 PM, Sriram
Natarajan<[email protected]> wrote:
> 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
>
> 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
>
>
> --
> Windows Internals Mailing List (http://www.php.net/)
> To unsubscribe, visit: http://www.php.net/unsub.php
>



-- 
Pierre

http://blog.thepimp.net | http://www.libgd.org