[ovs-dev] [PATCH 3/4] socket-util: errno for winsock related functions.

Ben Pfaff blp at nicira.com
Mon Feb 10 18:35:44 UTC 2014


On Mon, Feb 10, 2014 at 10:31:02AM -0800, Gurucharan Shetty wrote:
> On Thu, Feb 6, 2014 at 4:51 PM, Ben Pfaff <blp at nicira.com> wrote:
> > 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 <gshetty at nicira.com>
> >
> > 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?
> I don't yet see the need for the transformation for other error codes.
> 
> > 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.
> 
> This looks like the right thing to do. But stream-ssl.c uses EAGAIN
> for things other than just to check a return value. So, the makefile
> rule got a little tricky.

Hmm.  I expected it to be straightforward.  If it's too tricky, so
that we have to revise it often, then it may not be worth it.

> Since I see stream-ssl.c to be the only file where windows sockets and
> POSIX sockets co-exist, I decided to get rid of sock_errno() function
> from socket-util.c
> and moved it to stream-ssl.c. I also got rid of the two-way
> transformation of the error code and instead used a #ifdef directly
> after the accept() call. It seems to
> be the only place where we need this translation.

OK.  Thanks for thinking this through.

> >> +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?
> Yes. It can be done and v2 will have that change.

Great.  That will be easier to understand.

Thanks,

Ben.



More information about the dev mailing list