[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