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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



"Stephen R. van den Berg" <srb@xxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux