[Spice-devel] [PATCH spice-server 21/33] dispatcher: Port to Windows

[Frediano Ziglio]

> 
> Hi
> 
> On Fri, Dec 21, 2018 at 4:04 PM Frediano Ziglio <fziglio at redhat.com> wrote:
> >
> > Replace poll call with select.
> > As socket is set to non-blocking we must support it so if
> > we detect an EAGAIN error wait for data.
> 
> I am not fond of the two code paths, why not switch the code to select()?
> 

Not clear, new code is using select(). Or you mean all code, meaning Unix
and Windows? Unix does not suffer that issue as file descriptors remains
blocking, also select() suffer the 1024 limit and does not perform as
good as poll. Windows select() does not have the 1024 limit and the
performance are good (FDSETs in Windows are arrays).

> I wonder if existing poll() could be easily replaced with WSAPoll.
> 

Yes, see https://github.com/FreeTDS/freetds/blob/master/src/replacements/poll.c,
but looks overwhelming.

> 
> >
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/dispatcher.c | 20 ++++++++++++++++++++
> >  1 file changed, 20 insertions(+)
> >
> > diff --git a/server/dispatcher.c b/server/dispatcher.c
> > index 4cd91f11..9a55a4c9 100644
> > --- a/server/dispatcher.c
> > +++ b/server/dispatcher.c
> > @@ -197,6 +197,7 @@ static int read_safe(socket_t fd, uint8_t *buf, size_t
> > size, int block)
> >      }
> >
> >      if (!block) {
> > +#ifndef _WIN32
> >          struct pollfd pollfd = {.fd = socket_get_raw(fd), .events =
> >          POLLIN, .revents = 0};
> >          while ((ret = poll(&pollfd, 1, 0)) == -1) {
> >              if (errno == EINTR) {
> > @@ -209,6 +210,15 @@ static int read_safe(socket_t fd, uint8_t *buf, size_t
> > size, int block)
> >          if (!(pollfd.revents & POLLIN)) {
> >              return 0;
> >          }
> > +#else
> > +        struct timeval tv = { 0, 0 };
> > +        fd_set fds;
> > +        FD_ZERO(&fds);
> > +        FD_SET(socket_get_raw(fd), &fds);
> > +        if (select(1, &fds, NULL, NULL, &tv) < 1) {
> > +            return 0;
> > +        }
> > +#endif
> >      }
> >      while (read_size < size) {
> >          ret = socket_read(fd, buf + read_size, size - read_size);
> > @@ -217,6 +227,16 @@ static int read_safe(socket_t fd, uint8_t *buf, size_t
> > size, int block)
> >                  spice_debug("EINTR in read");
> >                  continue;
> >              }
> > +#ifdef _WIN32
> > +            // Windows turns this socket not-blocking
> > +            if (errno == EAGAIN) {
> > +                fd_set fds;
> > +                FD_ZERO(&fds);
> > +                FD_SET(socket_get_raw(fd), &fds);
> > +                select(1, &fds, NULL, NULL, NULL);
> > +                continue;
> > +            }
> > +#endif
> >              return -1;
> >          }
> >          if (ret == 0) {

Not saying that this patch is great.

Frediano