Re: [PATCH] git daemon: avoid calling syslog() from a signal handler

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jul 5, 2008, 10:05 UTC
Message-ID: <alpine.DEB.1.00.0807051201320.3334@eeepc-johanness>
In-Reply-To: <7vej68u6mr.fsf@gitster.siamese.dyndns.org>

Hi,

On Sat, 5 Jul 2008, Junio C Hamano wrote:

Show 30 quoted lines

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Signal handlers should never call syslog(), as that can raise signals 
> > of its own.
> >
> > Instead, call the syslog() from the master process.
> 
> Earlier parts seem to make sense but I am puzzled by these changes.
> 
> > @@ -929,7 +945,8 @@ static int service_loop(int socknum, int *socklist)
> >  	for (;;) {
> >  		int i;
> >  
> > -		if (poll(pfd, socknum, -1) < 0) {
> > +		i = poll(pfd, socknum, 1);
> > +		if (i < 0) {
> >  			if (errno != EINTR) {
> >  				error("poll failed, resuming: %s",
> >  				      strerror(errno));
> > @@ -937,6 +954,10 @@ static int service_loop(int socknum, int *socklist)
> >  			}
> >  			continue;
> >  		}
> > +		if (i == 0) {
> > +			check_dead_children();
> > +			continue;
> > +		}
> 
> So you will check every 1ms to see if there are new dead children, but why
> is this necessary?

This comes from me not reading the man page for poll() properly. Of course, I want to check every second: syslog timestamps the messages with a resolution of 1 second, AFAIR, or at least some of them do.

So if you could just squash in this patch, that would be smashing:

-- snipsnap --

@@ -945,8 +945,8 @@ static int service_loop(int socknum, int *socklist)
 	for (;;) {
 		int i;
 
-		i = poll(pfd, socknum, 1);
+		i = poll(pfd, socknum, 1000);
 		if (i < 0) {
 			if (errno != EINTR) {
 				error("poll failed, resuming: %s",
 				      strerror(errno));