Re: [PATCH] git-daemon: fix segfaulting in child_handler() in AIX

From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Apr 21, 2011, 20:43 UTC
Message-ID: <BANLkTinu0Rx9AM-+S8fyC97vUXXgeT5jbQ@mail.gmail.com>
In-Reply-To: <1302886260-25860-1-git-send-email-spartacus06@gmail.com>

On Fri, Apr 15, 2011 at 6:51 PM, Seth Jennings <spartacus06@gmail.com> wrote:

Show 8 quoted lines

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

Show 26 quoted lines

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