RE: poll() emulation in git

2012-09-07 Thread Joachim Schmitz 
 From: Paolo Bonzini [mailto:paolo.bonz...@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-gnu...@gnu.org; rsbec...@nexbridge.com
 Subject: Re: poll() emulation in git
 
 Il 06/09/2012 16:44, Joachim Schmitz ha scritto:
   Yes, it's an usleep(autocorrect * 10) 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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: poll() emulation in git

2012-09-07 Thread Joachim Schmitz 
 From: Paolo Bonzini [mailto:paolo.bonz...@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-gnu...@gnu.org; rsbec...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: poll() emulation in git

2012-09-07 Thread 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: poll() emulation in git

2012-09-06 Thread Joachim Schmitz 
 From: Paolo Bonzini [mailto:paolo.bonz...@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-gnu...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: poll() emulation in git

2012-09-06 Thread 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 * 10) 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: poll() emulation in git

2012-09-06 Thread Paolo Bonzini 
Il 06/09/2012 16:44, Joachim Schmitz ha scritto:
  Yes, it's an usleep(autocorrect * 10) 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: poll() emulation in git

2012-09-05 Thread Joachim Schmitz 
 From: Joachim Schmitz [mailto:j...@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:gits...@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 j...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: poll() emulation in git

2012-09-05 Thread 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: poll() emulation in git

2012-09-05 Thread Joachim Schmitz 
 From: Paolo Bonzini [mailto:paolo.bonz...@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-gnu...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: poll() emulation in git

2012-09-05 Thread Joachim Schmitz 
 From: Joachim Schmitz [mailto:j...@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-gnu...@gnu.org'
 Subject: RE: poll() emulation in git
 
  From: Paolo Bonzini [mailto:paolo.bonz...@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-gnu...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: poll() emulation in git

2012-09-05 Thread 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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html