Re: [Libguestfs] [PATCH nbdkit 7/9] server: Add hand-written replacement for poll for Windows.

Eric Blake
2020-08-18T13:31:42Z

On 8/18/20 5:50 AM, Richard W.M. Jones wrote:
> ---
>   configure.ac                    |   1 +
>   common/replacements/Makefile.am |   2 +
>   common/replacements/poll.h      |  60 ++++++++++++++++++
>   server/public.c                 |   2 +-
>   server/sockets.c                |   2 +-
>   common/replacements/poll.c      | 106 ++++++++++++++++++++++++++++++++
>   6 files changed, 171 insertions(+), 2 deletions(-)

My native windows coding is weak, so take this review with a grain of salt.

> +++ b/common/replacements/poll.c

> +#ifndef HAVE_POLL
> +
> +#include "poll.h"
> +
> +#ifdef WIN32
> +
> +#include <winsock2.h>
> +#include <ws2tcpip.h>
> +#include <windows.h>
> +
> +/* Windows doesn't have poll.  It has something called WSAPoll in
> + * Winsock, but even MSFT admit it is broken.  Gnulib contains an

Is MSFT considered plural, or should that be 'admits'

> + * elaborate emulation of poll written by Paolo, but it's distributed

I know who you mean, but is it worth a surname?

> + * under an incompatible license.  However Winsock has select so we
> + * can write a simple (but slow) emulation of poll using select.
> + */
> +int
> +poll (struct pollfd *fds, int n, int timeout)
> +{
> +  int i, nfds = 0, r;
> +  fd_set readfds, writefds;
> +  struct timeval tv, *tvp;
> +
> +  FD_ZERO (&readfds);
> +  FD_ZERO (&writefds);
> +
> +  for (i = 0; i < n; ++i) {
> +    if (fds[i].events & POLLIN)
> +      FD_SET (fds[i].fd, &readfds);
> +    if (fds[i].events & POLLOUT)
> +      FD_SET (fds[i].fd, &writefds);
> +    if (fds[i].fd > nfds)
> +      nfds = fds[i].fd;
> +    fds[i].revents = 0;
> +  }
> +  nfds++;

Do we need to make sure that nfds does not exceed the limits of select?
Calling FD_SET on a too-large fd causes memory corruption (twiddling
bits outside of the fixed-length bitset is never good).  Offhand, I
didn't know the windows bitset length, but a quick google found:

https://docs.microsoft.com/en-us/windows/win32/winsock/maximum-number-of-sockets-supported-2

where the default limit is 64 unless you set a compile-time macro prior
to winsock2.h.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org