Re: [PATCH 2/3] git-daemon: make the signal handler almost a no-op

[Junio C Hamano <gitster@pobox.com>]

"Stephen R. van den Berg" <srb@cuci.nl> writes:

> by exploiting the fact that systemcalls get interrupted by signals;
> and even they aren't, all zombies will be collected before the next
> accept().

Dscho may want to say something about "even they aren't..." part, after he
comes back to the keyboard.

> Fix another error() -> logerror() call.

which should have been in 1/3, I suppose.

> @@ -1036,10 +1034,7 @@ int main(int argc, char **argv)
>  	gid_t gid = 0;
>  	int i;
>  
> -	/* Without this we cannot rely on waitpid() to tell
> -	 * what happened to our children.
> -	 */
> -	signal(SIGCHLD, SIG_DFL);
> +	child_handler(0);

Why?

child_handler() logically is a two step process:

 * We are just informed that somebody died; let's do something about the
   corpse;

 * On some systems we need to rearm signals once they fired, so let's do
   that if necessary.

With your change, the first part happens to be almost no-op, but I do not
think it justifies this hunk.

After all, we might even want to do something like:

	static void child_handler(int signo)
        {
        	if (USE_SYSV_SIGNAL_SEMANTICS)
                	signal(SIGCHLD, child_handler);
	}

and have the compiler optimize out the signal rearming with

	cc CFLAGS=-DUSE_SYSV_SIGNAL_SEMANTICS=0

on suitable platforms in the future.  But you still want the initial
signal set-up to happen unconditionally.

At this point, we aren't informed by the system that somebody died, and we
would want to arm the signal regardless of the platform's signal semantics.

The rest of the patch looked sane, although I did not read it very
carefully.

Thanks.