RE: poll() emulation in git

From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
Sent: Thursday, September 06, 2012 4:32 PM
To: Joachim Schmitz
Cc: git@vger.kernel.org; 'Junio C Hamano'; 'Erik Faye-Lund'; bug-gnulib@gnu.org; rsbecker@nexbridge.com
Subject: Re: poll() emulation in git

Il 06/09/2012 16:02, Joachim Schmitz ha scritto:

quoted

But is there something that could be done to make git work even without poll()?
It is used in 5 places:

$ grep -n poll\( *.c */*.c
credential-cache--daemon.c:175: if (poll(&pfd, 1, 1000 * wakeup) < 0) {
daemon.c:1018:          if (poll(pfd, socklist->nr, -1) < 0) {
help.c:361:                     poll(NULL, 0, autocorrect * 100);
upload-pack.c:232:              if (poll(pfd, pollsize, -1) < 0) {
builtin/upload-archive.c:125:           if (poll(pfd, 2, -1) < 0) {

Don't quite understand why in help.c it has that NULL, which should always result in an EFAULT and other than that basically is

a

quoted

NOP (at least in the poll() emulation)? Seems a usleep(autocorrect * 100) is meant to happen here instead?
So I think here a poll() isn't needed at all. But also the 'broken' one shouldn't harm too much.

Yes, it's an usleep(autocorrect * 100000) basically (poll takes
milliseconds, not micro).

OK, it is _supposed_ to do this usleep(), but is does not, as poll() returns early with EFAULT in this case:
  /* EFAULT is not necessary to implement, but let's do it in the
     simplest case. */
  if (!pfd)
    {
      errno = EFAULT;
      return -1;
    }

poll() is doing this before calling select(), so won't sleep.
So there's a bug in {gnulib|git}'s poll(), right?

quoted

...
# else
      char data[64];
      r = recv (fd, data, sizeof (data), MSG_PEEK);
      socket_errno = (r < 0) ? errno : 0;
# endif
      if (r == 0)
        happened |= POLLHUP;

      /* If the event happened on an unconnected server socket,
         that's fine. */
      else if (r > 0 || ( /* (r == -1) && */ socket_errno == ENOTCONN))
        happened |= (POLLIN | POLLRDNORM) & sought;

      /* Distinguish hung-up sockets from other errors.  */
      else if (socket_errno == ESHUTDOWN || socket_errno == ECONNRESET
               || socket_errno == ECONNABORTED || socket_errno == ENETRESET)
        happened |= POLLHUP;

#ifdef __TANDEM /* as we can't recv(...,MSG_PEEK) on a non-socket */
      else if (socket_errno == ENOTSOCK)
        happened |= (POLLIN | POLLRDNORM) & sought;
#endif
      else
        happened |= POLLERR;
    }
...

We won't detect POLLHUP that way I think. However it seems to work, we've been able to clone, push, pull, branch that way with
NonStop being the (ssh-)server, something that didn't work at all without that hack (and yes, I believe it is just that).
Someone in for a cleaner way of managing this?

I suppose it works to always handle ENOTSOCK that way, even on
non-__TANDEM systems.

So you think this is a clean way of dealing with it?

Bye, Jojo