From: Erik Faye-Lund Date: Thu, 21 Apr 2011 20:43:26 GMT Subject: Re: [PATCH] git-daemon: fix segfaulting in child_handler() in AIX Message-ID: In-Reply-To: <1302886260-25860-1-git-send-email-spartacus06@gmail.com> On Fri, Apr 15, 2011 at 6:51 PM, Seth Jennings wrote: > This issue seems to be specific to git-daemon on AIX built with xlc. > After commit 695605b5080e1957bd9dab1fed35a7fee9814297 (from Aug 2008), > git-daemon segfaults in child_handler() inside the signal() syscall > immediately after any remote clone/pull operation.  While it is not > fully understood why this happens, changing signal() to sigaction() > resolves the issue. > > This commit converts singal() to sigaction() in child_handler(). At first I thought that a change like this would break the Windows-port, because our win32-implementation of sigaction does not support SIGCHLD. But looking at the code (and MSDN), our win32-implementation of signal doesn't either. However, our signal-implementation piggy-backs on the signal-implementation in msvcrt.dll for non-SIGALRM signals, and microsoft's implementation does not seem to update errno in this case. So, this patch will introduce an error-line saying "sigaction only implemented for SIGALRM". Exactly what SHOULD be done for Windows in this case isn't entirely clear. Perhaps we should just ignore it? Apart from this, I must say I agree with Christian; we should find out exactly why a change like this is needed. > --- >  daemon.c |   10 ++++++++-- >  1 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/daemon.c b/daemon.c > index 4c8346d..3ea5b2c 100644 > --- a/daemon.c > +++ b/daemon.c > @@ -715,7 +715,10 @@ static void child_handler(int signo) >         * upon signal receipt >         * SysV needs the handler to be rearmed >         */ > -       signal(SIGCHLD, child_handler); > +       struct sigaction sigact; > +       memset(&sigact, 0, sizeof(sigact)); > +       sigact.sa_handler = child_handler; > +       sigaction(SIGCHLD, &sigact, NULL); >  } > >  static int set_reuse_addr(int sockfd) > @@ -889,7 +892,10 @@ static int service_loop(struct socketlist *socklist) >                pfd[i].events = POLLIN; >        } > > -       signal(SIGCHLD, child_handler); > +       struct sigaction sigact; This gives a warning on MinGW: daemon.c: In function 'service_loop': daemon.c:895: warning: ISO C90 forbids mixed declarations and code