RE: poll() emulation in git

RE: poll() emulation in git

[Joachim Schmitz]

> From: Joachim Schmitz [mailto:jojo@schmitz-digital.de]
> Sent: Tuesday, September 04, 2012 1:49 PM
> To: 'Junio C Hamano'
> Cc: 'git@vger.kernel.org'; 'Erik Faye-Lund'
> Subject: RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
> 
> > From: Junio C Hamano [mailto:gitster@pobox.com]
> > Sent: Friday, August 24, 2012 9:47 PM
> > To: Joachim Schmitz
> > Cc: git@vger.kernel.org; 'Erik Faye-Lund'
> > Subject: Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
> >
> > "Joachim Schmitz" <jojo@schmitz-digital.de> writes:
> >
> > > Different, but related question: would poll.[ch] be allowed to #include "git-compat-util.h"?
> >
> > Seeing other existing generic wrappers directly under compat/,
> > e.g. fopen.c, mkdtemp.c, doing so, I would say why not.
> >
> > Windows folks (I see Erik is already CC'ed, which is good ;-),
> > please work with Joachim to make sure such a move won't break your
> > builds.  I believe that it should just be the matter of updating a
> > couple of paths in the top-level Makefile.
> 
> Haven't heard anything from the Windows folks yet.
> 
> I'd prefer to move compat/win32/poll.[ch] into compat/poll.
> Then adjust a few paths in Makefile and that would be the 1st patch
> 
> A 2nd patch would be my already proposed ones that make this usable for others (me in this case ;-)), namely wrapping 2 #inludes.
> 
> diff --git a/compat/poll/poll.c b/compat/poll/poll.c
> index 403eaa7..49541f1 100644
> --- a/compat/poll/poll.c
> +++ b/compat/poll/poll.c
> @@ -24,7 +24,9 @@
>  # pragma GCC diagnostic ignored "-Wtype-limits"
>  #endif
> 
> -#include <malloc.h>
> +#if defined(WIN32)
> +# include <malloc.h>
> +#endif
> 
>  #include <sys/types.h>
> 
> @@ -48,7 +50,9 @@
>  #else
>  # include <sys/time.h>
>  # include <sys/socket.h>
> -# include <sys/select.h>
> +# ifndef NO_SYS_SELECT_H
> +#  include <sys/select.h>
> +# endif
>  # include <unistd.h>
>  #endif
> 
> --
> 1.7.12

However: this poll implementation, while compiling OK, doesn't work properly.
Because it uses recv(...,MSG_PEEK), it works on sockets only (returns ENOTSOCK on anything else), while the real poll() works on all
kind if file descriptors, at least that is my understanding.

Here on HP NonStop, when being connected via an non-interactive SSH, we get a set of pipes (stdin, stdout, stderr) instead of a
socket to talk to, so the poll() just hangs/loops.

As git's implementation is based on ('stolen' from?) gnulib's and still pretty similar, CC to the gnulib list and Paolo

Any idea how this could get solved? I.e. how to implement a poll() that works on non-sockets too?
There is some code that pertains to a seemingly similar problem in Mac OS X, but my problem is not identical, as that fix doesn't
help.

Bye, Jojo

Re: poll() emulation in git

[Bastien ROUCARIES]

On Wed, Sep 5, 2012 at 1:24 PM, Joachim Schmitz <jojo@schmitz-digital.de> wrote:
>> From: Joachim Schmitz [mailto:jojo@schmitz-digital.de]
>> Sent: Tuesday, September 04, 2012 1:49 PM
>> To: 'Junio C Hamano'
>> Cc: 'git@vger.kernel.org'; 'Erik Faye-Lund'
>> Subject: RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
>>
>> > From: Junio C Hamano [mailto:gitster@pobox.com]
>> > Sent: Friday, August 24, 2012 9:47 PM
>> > To: Joachim Schmitz
>> > Cc: git@vger.kernel.org; 'Erik Faye-Lund'
>> > Subject: Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
>> >
>> > "Joachim Schmitz" <jojo@schmitz-digital.de> writes:
>> >
>> > > Different, but related question: would poll.[ch] be allowed to #include "git-compat-util.h"?
>> >
>> > Seeing other existing generic wrappers directly under compat/,
>> > e.g. fopen.c, mkdtemp.c, doing so, I would say why not.
>> >
>> > Windows folks (I see Erik is already CC'ed, which is good ;-),
>> > please work with Joachim to make sure such a move won't break your
>> > builds.  I believe that it should just be the matter of updating a
>> > couple of paths in the top-level Makefile.
>>
>> Haven't heard anything from the Windows folks yet.
>>
>> I'd prefer to move compat/win32/poll.[ch] into compat/poll.
>> Then adjust a few paths in Makefile and that would be the 1st patch
>>
>> A 2nd patch would be my already proposed ones that make this usable for others (me in this case ;-)), namely wrapping 2 #inludes.
>>
>> diff --git a/compat/poll/poll.c b/compat/poll/poll.c
>> index 403eaa7..49541f1 100644
>> --- a/compat/poll/poll.c
>> +++ b/compat/poll/poll.c
>> @@ -24,7 +24,9 @@
>>  # pragma GCC diagnostic ignored "-Wtype-limits"
>>  #endif
>>
>> -#include <malloc.h>
>> +#if defined(WIN32)
>> +# include <malloc.h>
>> +#endif
>>
>>  #include <sys/types.h>
>>
>> @@ -48,7 +50,9 @@
>>  #else
>>  # include <sys/time.h>
>>  # include <sys/socket.h>
>> -# include <sys/select.h>
>> +# ifndef NO_SYS_SELECT_H
>> +#  include <sys/select.h>
>> +# endif
>>  # include <unistd.h>
>>  #endif
>>
>> --
>> 1.7.12
>
> However: this poll implementation, while compiling OK, doesn't work properly.
> Because it uses recv(...,MSG_PEEK), it works on sockets only (returns ENOTSOCK on anything else), while the real poll() works on all
> kind if file descriptors, at least that is my understanding.
>
> Here on HP NonStop, when being connected via an non-interactive SSH, we get a set of pipes (stdin, stdout, stderr) instead of a
> socket to talk to, so the poll() just hangs/loops.
>
> As git's implementation is based on ('stolen' from?) gnulib's and still pretty similar, CC to the gnulib list and Paolo
>
> Any idea how this could get solved? I.e. how to implement a poll() that works on non-sockets too?
> There is some code that pertains to a seemingly similar problem in Mac OS X, but my problem is not identical, as that fix doesn't
> help.

Could you give more context ? Are you speaking about win32 or about HP non stop?

If so pipe are broken and unfixable under windows see
http://www.mail-archive.com/bug-gnulib@gnu.org/msg23365.html

> Bye, Jojo
>
>

RE: poll() emulation in git

[Joachim Schmitz]

> From: Bastien ROUCARIES [mailto:roucaries.bastien@gmail.com]
> Sent: Wednesday, September 05, 2012 1:55 PM
> To: Joachim Schmitz
> Cc: Junio C Hamano; Paolo Bonzini; bug-gnulib@gnu.org; git@vger.kernel.org; Erik Faye-Lund
> Subject: Re: poll() emulation in git
> 
> On Wed, Sep 5, 2012 at 1:24 PM, Joachim Schmitz <jojo@schmitz-digital.de> wrote:
> >> From: Joachim Schmitz [mailto:jojo@schmitz-digital.de]
> >> Sent: Tuesday, September 04, 2012 1:49 PM
> >> To: 'Junio C Hamano'
> >> Cc: 'git@vger.kernel.org'; 'Erik Faye-Lund'
> >> Subject: RE: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
> >>
> >> > From: Junio C Hamano [mailto:gitster@pobox.com]
> >> > Sent: Friday, August 24, 2012 9:47 PM
> >> > To: Joachim Schmitz
> >> > Cc: git@vger.kernel.org; 'Erik Faye-Lund'
> >> > Subject: Re: [PATCH v2] Support non-WIN32 system lacking poll() while keeping the WIN32 part intact
> >> >
> >> > "Joachim Schmitz" <jojo@schmitz-digital.de> writes:
> >> >
> >> > > Different, but related question: would poll.[ch] be allowed to #include "git-compat-util.h"?
> >> >
> >> > Seeing other existing generic wrappers directly under compat/,
> >> > e.g. fopen.c, mkdtemp.c, doing so, I would say why not.
> >> >
> >> > Windows folks (I see Erik is already CC'ed, which is good ;-),
> >> > please work with Joachim to make sure such a move won't break your
> >> > builds.  I believe that it should just be the matter of updating a
> >> > couple of paths in the top-level Makefile.
> >>
> >> Haven't heard anything from the Windows folks yet.
> >>
> >> I'd prefer to move compat/win32/poll.[ch] into compat/poll.
> >> Then adjust a few paths in Makefile and that would be the 1st patch
> >>
> >> A 2nd patch would be my already proposed ones that make this usable for others (me in this case ;-)), namely wrapping 2 #inludes.
> >>
> >> diff --git a/compat/poll/poll.c b/compat/poll/poll.c
> >> index 403eaa7..49541f1 100644
> >> --- a/compat/poll/poll.c
> >> +++ b/compat/poll/poll.c
> >> @@ -24,7 +24,9 @@
> >>  # pragma GCC diagnostic ignored "-Wtype-limits"
> >>  #endif
> >>
> >> -#include <malloc.h>
> >> +#if defined(WIN32)
> >> +# include <malloc.h>
> >> +#endif
> >>
> >>  #include <sys/types.h>
> >>
> >> @@ -48,7 +50,9 @@
> >>  #else
> >>  # include <sys/time.h>
> >>  # include <sys/socket.h>
> >> -# include <sys/select.h>
> >> +# ifndef NO_SYS_SELECT_H
> >> +#  include <sys/select.h>
> >> +# endif
> >>  # include <unistd.h>
> >>  #endif
> >>
> >> --
> >> 1.7.12
> >
> > However: this poll implementation, while compiling OK, doesn't work properly.
> > Because it uses recv(...,MSG_PEEK), it works on sockets only (returns ENOTSOCK on anything else), while the real poll() works on all
> > kind if file descriptors, at least that is my understanding.
> >
> > Here on HP NonStop, when being connected via an non-interactive SSH, we get a set of pipes (stdin, stdout, stderr) instead of a
> > socket to talk to, so the poll() just hangs/loops.
> >
> > As git's implementation is based on ('stolen' from?) gnulib's and still pretty similar, CC to the gnulib list and Paolo
> >
> > Any idea how this could get solved? I.e. how to implement a poll() that works on non-sockets too?
> > There is some code that pertains to a seemingly similar problem in Mac OS X, but my problem is not identical, as that fix doesn't
> > help.
> 
> Could you give more context ? Are you speaking about win32 or about HP non stop?

HP NonStop

> If so pipe are broken and unfixable under windows see
> http://www.mail-archive.com/bug-gnulib@gnu.org/msg23365.html

Bye, Jojo

Re: poll() emulation in git

[Paolo Bonzini]

Il 05/09/2012 13:24, Joachim Schmitz ha scritto:
> However: this poll implementation, while compiling OK, doesn't work properly.
> Because it uses recv(...,MSG_PEEK), it works on sockets only (returns ENOTSOCK on anything else), while the real poll() works on all
> kind if file descriptors, at least that is my understanding.

Actually recv(...,MSG_PEEK) on most Unix variants works on non-sockets
too.  The trick is taken from GNU Pth in turn.

> Here on HP NonStop, when being connected via an non-interactive SSH, we get a set of pipes (stdin, stdout, stderr) instead of a
> socket to talk to, so the poll() just hangs/loops.

Does your system have a working FIONREAD ioctl for pipes?

> As git's implementation is based on ('stolen' from?) gnulib's and still pretty similar, CC to the gnulib list and Paolo
> 
> Any idea how this could get solved? I.e. how to implement a poll() that works on non-sockets too?
> There is some code that pertains to a seemingly similar problem in Mac OS X, but my problem is not identical, as that fix doesn't
> help.

Paolo

RE: poll() emulation in git

[Joachim Schmitz]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Wednesday, September 05, 2012 2:05 PM
> To: Joachim Schmitz
> Cc: 'Junio C Hamano'; git@vger.kernel.org; 'Erik Faye-Lund'; bug-gnulib@gnu.org
> Subject: Re: poll() emulation in git
> 
> Il 05/09/2012 13:24, Joachim Schmitz ha scritto:
> > However: this poll implementation, while compiling OK, doesn't work properly.
> > Because it uses recv(...,MSG_PEEK), it works on sockets only (returns ENOTSOCK on anything else), while the real poll() works on
all
> > kind if file descriptors, at least that is my understanding.
> 
> Actually recv(...,MSG_PEEK) on most Unix variants works on non-sockets
> too.  The trick is taken from GNU Pth in turn.
> 
> > Here on HP NonStop, when being connected via an non-interactive SSH, we get a set of pipes (stdin, stdout, stderr) instead of a
> > socket to talk to, so the poll() just hangs/loops.
> 
> Does your system have a working FIONREAD ioctl for pipes?

It does have FIONREAD ioctl. Whether it works properly is to be determined...
I'll test if you could show me how?


Bye, Jojo

RE: poll() emulation in git

[Joachim Schmitz]

> From: Joachim Schmitz [mailto:jojo@schmitz-digital.de]
> Sent: Wednesday, September 05, 2012 2:58 PM
> To: 'Paolo Bonzini'
> Cc: 'Junio C Hamano'; 'git@vger.kernel.org'; 'Erik Faye-Lund'; 'bug-gnulib@gnu.org'
> Subject: RE: poll() emulation in git
> 
> > From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> > Sent: Wednesday, September 05, 2012 2:05 PM
> > To: Joachim Schmitz
> > Cc: 'Junio C Hamano'; git@vger.kernel.org; 'Erik Faye-Lund'; bug-gnulib@gnu.org
> > Subject: Re: poll() emulation in git
> >
> > Il 05/09/2012 13:24, Joachim Schmitz ha scritto:
> > > However: this poll implementation, while compiling OK, doesn't work properly.
> > > Because it uses recv(...,MSG_PEEK), it works on sockets only (returns ENOTSOCK on anything else), while the real poll() works
on all
> > > kind if file descriptors, at least that is my understanding.
> >
> > Actually recv(...,MSG_PEEK) on most Unix variants works on non-sockets
> > too.  The trick is taken from GNU Pth in turn.
> >
> > > Here on HP NonStop, when being connected via an non-interactive SSH, we get a set of pipes (stdin, stdout, stderr) instead of
a
> > > socket to talk to, so the poll() just hangs/loops.
> >
> > Does your system have a working FIONREAD ioctl for pipes?
> 
> It does have FIONREAD ioctl. Whether it works properly is to be determined...
> I'll test if you could show me how?

Oh, now I see what you aimed at, but no, that Mac OS X method doesn't work for me, I tried (at least I think I did).

And <sys/ioctl.h> has
/*
 * Normal IOCTL's supported by the socket interface
 */
#define FIONREAD        _IOR(0, 8, _ioctl_int)       /* Num of bytes to read */
#define FIONBIO         _IOW(0, 9, _ioctl_int)       /* Non-blocking I/O     */

So these seem to be supported on sockets only, I guess.
And indeed the man pages for ioctl confirms:

          Valid values for the request parameter for AF_INET or
          AF_INET6 sockets are:


          FIONREAD  Gets the number of bytes available for reading and
                    stores it at the int pointed at by arg.


So not even AF_UNIX sockets, not to mention pipes...

Bye, Jojo

Re: poll() emulation in git

[Paolo Bonzini]

Il 05/09/2012 15:36, Joachim Schmitz ha scritto:
>>> > > Does your system have a working FIONREAD ioctl for pipes?
>> > 
>> > It does have FIONREAD ioctl. Whether it works properly is to be determined...
>> > I'll test if you could show me how?
> Oh, now I see what you aimed at, but no, that Mac OS X method doesn't work for me, I tried (at least I think I did).
> 
> And <sys/ioctl.h> has
> /*
>  * Normal IOCTL's supported by the socket interface
>  */
> #define FIONREAD        _IOR(0, 8, _ioctl_int)       /* Num of bytes to read */
> #define FIONBIO         _IOW(0, 9, _ioctl_int)       /* Non-blocking I/O     */
> 
> So these seem to be supported on sockets only, I guess.
> And indeed the man pages for ioctl confirms:
> 
>           Valid values for the request parameter for AF_INET or
>           AF_INET6 sockets are:
> 
> 
>           FIONREAD  Gets the number of bytes available for reading and
>                     stores it at the int pointed at by arg.
> 
> 
> So not even AF_UNIX sockets, not to mention pipes...

So there's no way you can support POLLHUP.  Your system is quite
crippled. :(

Paolo

RE: poll() emulation in git

[Joachim Schmitz]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Wednesday, September 05, 2012 5:26 PM
> To: Joachim Schmitz
> Cc: 'Junio C Hamano'; git@vger.kernel.org; 'Erik Faye-Lund'; bug-gnulib@gnu.org
> Subject: Re: poll() emulation in git
> 
> Il 05/09/2012 15:36, Joachim Schmitz ha scritto:
> >>> > > Does your system have a working FIONREAD ioctl for pipes?
> >> >
> >> > It does have FIONREAD ioctl. Whether it works properly is to be determined...
> >> > I'll test if you could show me how?
> > Oh, now I see what you aimed at, but no, that Mac OS X method doesn't work for me, I tried (at least I think I did).
> >
> > And <sys/ioctl.h> has
> > /*
> >  * Normal IOCTL's supported by the socket interface
> >  */
> > #define FIONREAD        _IOR(0, 8, _ioctl_int)       /* Num of bytes to read */
> > #define FIONBIO         _IOW(0, 9, _ioctl_int)       /* Non-blocking I/O     */
> >
> > So these seem to be supported on sockets only, I guess.
> > And indeed the man pages for ioctl confirms:
> >
> >           Valid values for the request parameter for AF_INET or
> >           AF_INET6 sockets are:
> >
> >
> >           FIONREAD  Gets the number of bytes available for reading and
> >                     stores it at the int pointed at by arg.
> >
> >
> > So not even AF_UNIX sockets, not to mention pipes...
> 
> So there's no way you can support POLLHUP.  Your system is quite
> crippled. :(

Unfortunatly.

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
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.

In daemon.c it seems to be all sockets it polls on, so it should work on NonStop.
Same in credential-cache--daemon.c

Remains upload-pack.c and builtin/upload-archive.c
In both start_command() gathers the FDs to poll() on and that indeed works on pipes -> problem on NonStop!

Seeing that in those cases xread() takes care of EAGAIN, I've now used 'brute force' in poll.c:

...
# 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?

Bye, Jojo

Re: poll() emulation in git

[Paolo Bonzini]

Il 06/09/2012 16:02, Joachim Schmitz ha scritto:
> 
> 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
> 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).

> ...
> # 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.

Paolo

RE: poll() emulation in git

[Joachim Schmitz]

> 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:
> >
> > 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
> > 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?

> > ...
> > # 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

Re: poll() emulation in git

[Paolo Bonzini]

Il 06/09/2012 16:44, Joachim Schmitz ha scritto:
>> > 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?
> 

Yes, it should be "if (!pfd && nfd)".

Paolo

RE: poll() emulation in git

[Joachim Schmitz]

> From: Paolo Bonzini [mailto:paolo.bonzini@gmail.com] On Behalf Of Paolo Bonzini
> Sent: Thursday, September 06, 2012 5:15 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:44, Joachim Schmitz ha scritto:
> >> > 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?
> >
> 
> Yes, it should be "if (!pfd && nfd)".

Are you going to fix this in gnulib?

Bye, Jojo

RE: poll() emulation in git

[Joachim Schmitz]

> 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:
> > ...
> > # 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.

Will you be fixing this in gnulib? How?

Bye, Jojo

Re: poll() emulation in git

[Paolo Bonzini]

Il 07/09/2012 09:39, Joachim Schmitz ha scritto:
>> > I suppose it works to always handle ENOTSOCK that way, even on
>> > non-__TANDEM systems.
> Will you be fixing this in gnulib? How?

I don't have access to the system, so it's best if you post the patches
yourself to bug-gnulib and git mailing lists (separately, since the code
is cross-pollinated but forked).

Paolo