On Thu, Feb 06, 2014 at 08:12:32AM -0800, Gurucharan Shetty wrote:
> For Windows platform, there are different ways to get error numbers
> for differnt situations. For C library functions, errno is set like the way
> they are set for Linux. For Windows API functions, it has to be gotten from
> GetLastError().  For winsock2, errno has to be obtained from 
> WSAGetLastError().
> 
> The new functions will have the first users in an upcoming commit.
> 
> Signed-off-by: Gurucharan Shetty <[email protected]>

I'm a little uncomfortable with this two-way transformation.  It seems
like it will be easy to get it wrong.  Will we need to transform other
error codes too?  If not, then it might be better to add a function
like this:

    bool
    is_eagain(int error)
    {
        return (error == EAGAIN || error == EWOULDBLOCK
    #ifdef WSAEWOULDBLOCK
                || error == WASEWOULDBLOCK
    #endif
               );
    }

and then we could try to enforce its use with a Makefile rule.

I'm inclined to think that, if we want to transform error codes like
this, then we should define wrappers for all the common functions that
are likely to need them, something like this:

    int
    do_connect(int socket, struct sockaddr *addr, socklen_t length)
    {
        return connect(socket, addr, length) == -1 ? sock_errno() : 0;
    }

and again then try to enforce their use via Makefile rule.

Some comments below.

> +/* In Windows platform, errno is not set for socket calls.
> + * The last error has to be gotten from WSAGetLastError(). */
> +int

This should say (void) instead of ():

> +sock_errno()
> +{
> +#ifdef _WIN32
> +    int error = WSAGetLastError();
> +
> +    /* In case of non-blocking sockets, EAGAIN is an error that is checked to
> +     * retry. To prevent multiple #ifdef's across the code base, set the 
> error
> +     * as EAGAIN when it is WSAEWOULDBLOCK. */
> +    if (error == WSAEWOULDBLOCK) {
> +        error = EAGAIN;
> +    }
> +    return error;
> +#else
> +    return errno;
> +#endif
> +}

Can we make sock_strerror() do something like ovs_strerror()
internally does, to eliminate the need for the caller to free its
return value?

Thanks,

Ben.
_______________________________________________
dev mailing list
[email protected]
http://openvswitch.org/mailman/listinfo/dev

Reply via email to