[pacman-dev] [PATCH] pacman: Correct signal handler behavior

[Silvan Jegen]

On Fri, Jun 20, 2014 at 05:12:10PM -0400, Andrew Gregory wrote:
> On 06/20/14 at 10:00pm, Silvan Jegen wrote:
> > One of the comments for this function is out of sync with the code.
> > If a SIGHUP is received the code calls the release function only if
> > there is no transaction in flight (contrary to the comments).
> > 
> > This change brings the code in line with the comment (and moves the
> > comment for readability). Additionally, it changes the exit message
> > to reflect the fact that the signal received was either a SIGHUP or
> > a SIGTERM.
> > 
> > Signed-off-by: Silvan Jegen <s.jegen at gmail.com>
> > ---
> >  src/pacman/pacman.c | 18 +++++++++---------
> >  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> I think the comment is just a typo.  According to its commit message,
> SIGHUP and SIGINT should be treated the same.  SIGHUP seems like
> a poor reason to kill an active transaction rather than more
> gracefully interrupt it.

I tend to agree.

Should I send a patch to change the comment accordingly (and maybe move
it before the "else if(signum == SIGINT || signum == SIGHUP) {" in the
original code)?

> apg
> 
> > diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c
> > index ef86d39..599a141 100644
> > --- a/src/pacman/pacman.c
> > +++ b/src/pacman/pacman.c
> > @@ -301,20 +301,20 @@ static void handler(int signum)
> >  			"Please submit a full bug report with --debug if appropriate.\n";
> >  		xwrite(err, msg, strlen(msg));
> >  		exit(signum);
> > -	} else if(signum == SIGINT || signum == SIGHUP) {
> > -		if(signum == SIGINT) {
> > -			msg = "\nInterrupt signal received\n";
> > -		} else {
> > -			msg = "\nHangup signal received\n";
> > -		}
> > -		xwrite(err, msg, strlen(msg));
> > +	}
> > +	/* SIGINT: no committing transaction, release it now and then exit pacman
> > +	 * SIGHUP, SIGTERM: release no matter what */
> > +	if(signum == SIGINT) {
> > +		msg = "\nInterrupt signal received\n";
> >  		if(alpm_trans_interrupt(config->handle) == 0) {
> >  			/* a transaction is being interrupted, don't exit pacman yet. */
> >  			return;
> >  		}
> > +	} else {
> > +		msg = "\nHangup/Termination signal received\n";
> >  	}
> > -	/* SIGINT: no committing transaction, release it now and then exit pacman
> > -	 * SIGHUP, SIGTERM: release no matter what */
> > +
> > +	xwrite(err, msg, strlen(msg));
> >  	alpm_trans_release(config->handle);
> >  	/* output a newline to be sure we clear any line we may be on */
> >  	xwrite(out, "\n", 1);
> > -- 
> > 2.0.0
>