Re: [libvirt PATCH v2 06/13] util: introduce compat wrappers for Winsock2

On Thu, Jan 16, 2020 at 03:24:41PM +0000, Daniel P. Berrangé wrote: > Windows sockets take a SOCKET HANDLE object instead of a > file descriptor. Wrap them in the same way that gnulib > does so that they use C runtime file descriptors. > > While we could in theory use GSocket, it is hard to get > the exact same semantics libvirt has for its current > socket usage. Wrapping the Winsock2 APIs is thus the > easiest approach in the short term. > > Reviewed-by: Pavel Hrdina <phrdina(a)redhat.com&gt; > Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; > --- > src/util/Makefile.inc.am | 2 + > src/util/virsocket.c | 369 +++++++++++++++++++++++++++++++++++++++ > src/util/virsocket.h | 92 ++++++++++ > src/util/virutil.c | 29 ++- > 4 files changed, 488 insertions(+), 4 deletions(-) > create mode 100644 src/util/virsocket.c > create mode 100644 src/util/virsocket.h > > diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am > index 23de4a6375..528c9f6cfe 100644 > --- a/src/util/Makefile.inc.am > +++ b/src/util/Makefile.inc.am > @@ -188,6 +188,8 @@ UTIL_SOURCES = \ > util/virseclabel.h \ > util/virsecret.c \ > util/virsecret.h \ > + util/virsocket.c \ > + util/virsocket.h \ > util/virsocketaddr.c \ > util/virsocketaddr.h \ > util/virstorageencryption.c \ > diff --git a/src/util/virsocket.c b/src/util/virsocket.c > new file mode 100644 > index 0000000000..2d770b931b > --- /dev/null > +++ b/src/util/virsocket.c > @@ -0,0 +1,369 @@ > +/* > + * Copyright (C) 2020 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>;. > + */ > + > +#include <config.h> > + > +#include "virsocket.h" > + > +#ifdef WIN32 > + > +# include <fcntl.h> > + > +# define FD2SK(fd) _get_osfhandle(fd) > +# define SK2FD(sk) (_open_osfhandle((intptr_t) (sk), O_RDWR | O_BINARY)) > + > +# define GET_HANDLE(fd) \ > + > +# define RETURN_ERROR(call) \ > + if ((call) < 0) { \ > + set_errno(); \ > + return -1; \ > + } > + > +# undef accept > +# undef bind > +# undef closesocket > +# undef connect > +# undef dup > +# undef dup2 We are not calling these two here so probably no need to undef here. > +# undef getpeername > +# undef getsockname > +# undef getsockopt > +# undef ioctlsocket > +# undef listen > +# undef setsockopt > +# undef socket [...] > diff --git a/src/util/virsocket.h b/src/util/virsocket.h > new file mode 100644 > index 0000000000..e9ef5380a3 > --- /dev/null > +++ b/src/util/virsocket.h > @@ -0,0 +1,92 @@ > +/* > + * Copyright (C) 2020 Red Hat, Inc. > + * > + * This library is free software; you can redistribute it and/or > + * modify it under the terms of the GNU Lesser General Public > + * License as published by the Free Software Foundation; either > + * version 2.1 of the License, or (at your option) any later version. > + * > + * This library is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + * Lesser General Public License for more details. > + * > + * You should have received a copy of the GNU Lesser General Public > + * License along with this library. If not, see > + * <http://www.gnu.org/licenses/>;. > + */ > + > +#pragma once > + > +#include "internal.h" > + > +#ifdef WIN32 > + > +# define WIN32_LEAN_AND_MEAN > +# include <errno.h> > +# include <winsock2.h> > +# include <ws2tcpip.h> > +# include <io.h> > + > +int vir_accept(int fd, struct sockaddr *addr, socklen_t *addrlen); > +int vir_bind(int fd, const struct sockaddr *addr, socklen_t addrlen); > +int vir_closesocket(int fd); > +int vir_connect(int fd, const struct sockaddr *addr, socklen_t addrlen); > +int vir_dup(int oldfd); > +int vir_dup2(int oldfd, int newfd); These two are not implemented anywhere as you are using the _dup and _dup2 from io.h.

> +int vir_getpeername(int fd, struct sockaddr *addr, socklen_t *addrlen); > +int vir_getsockname(int fd, struct sockaddr *addr, socklen_t *addrlen); > +int vir_listen(int fd, int backlog); > +int vir_ioctlsocket(int fd, int cmd, void *arg); > +int vir_getsockopt(int fd, int level, int optname, > + void *optval, socklen_t *optlen); > +int vir_setsockopt(int fd, int level, int optname, > + const void *optval, socklen_t optlen); > +int vir_socket(int domain, int type, int protocol); One nit that I missed in v1, the function declaration doesn't follow syntax style where each argument should be on separate line, but I don't care that much for these wrappers.

> + > + > +/* Get rid of GNULIB's replacements */ > +# undef accept > +# undef bind > +# undef closesocket > +# undef connect > +# undef dup > +# undef dup2 > +# undef getpeername > +# undef getsockname > +# undef getsockopt > +# undef ioctlsocket > +# undef listen > +# undef setsockopt > +# undef socket > + > +/* Provide our own replacements */ > +# define accept vir_accept > +# define bind vir_bind > +# define closesocket vir_closesocket > +# define connect vir_connect > +# define dup _dup > +# define dup2 _dup2 > +# define ioctlsocket vir_ioctlsocket > +# define getpeername vir_getpeername > +# define getsockname vir_getsockname > +# define getsockopt vir_getsockopt > +# define listen vir_listen > +# define setsockopt vir_setsockopt > +# define socket vir_socket > + > +#else > + > +# include <sys/socket.h> > +# include <unistd.h> > +# include <sys/ioctl.h> > +# include <arpa/inet.h> > +# include <netinet/in.h> > +# include <netinet/udp.h> > +# include <netinet/tcp.h> > +# include <sys/un.h> > + > +# define closesocket close > +# define ioctlsocket ioctl > + > +#endif > diff --git a/src/util/virutil.c b/src/util/virutil.c > index a0fd7618ee..9ea9e2946d 100644 > --- a/src/util/virutil.c > +++ b/src/util/virutil.c Changes to this should be in a separate patch but otherwise it looks good.