[PATCH 3 of 3] chg: forward SIGINT, SIGHUP to process group

Yuya Nishihara yuya at tcha.org
Mon Jul 18 06:06:41 EDT 2016


On Sun, 17 Jul 2016 23:39:54 +0100, Jun Wu wrote:
> Excerpts from Jun Wu's message of 2016-07-17 23:09:43 +0100:
> > -static void setupsignalhandler(pid_t pid)
> > +static void setupsignalhandler(const hgclient_t *hgc)
> >  {
> > -    if (pid <= 0)
> > +    peerpgid = hgc_peerpgid(hgc);
> > +    peerpid = hgc_peerpid(hgc);  
> 
> I noticed this changed behavior a bit as I pursued short code: the second
> setupsignalhandler call (if exists) becomes unsafe and the if condition is
> not quite right. It's probably better not to save lines:
> 
> diff --git a/contrib/chg/chg.c b/contrib/chg/chg.c
> --- a/contrib/chg/chg.c
> +++ b/contrib/chg/chg.c
> @@ -404,10 +404,14 @@ static void handlechildsignal(int sig UN
>  
>  static void setupsignalhandler(const hgclient_t *hgc)
>  {
> -       peerpgid = hgc_peerpgid(hgc);
> -       peerpid = hgc_peerpid(hgc);
> -       if (peerpgid <= 0 && peerpid <= 0)
> +       pid_t pid = hgc_peerpid(hgc);
> +       if (pid <= 0)
>                 return;
> +       pid_t pgid = hgc_peerpgid(hgc);
> +       if (pgid < 0)
> +               pgid = 0;
> +       peerpid = pid;
> +       peerpgid = pgid;

Yeah, something like that. Also, we should check pgid > 1 because kill(-1, sig)
would kill all user processes.

 static void forwardsignaltogroup(int sig)
 {
+	assert(peerpid > 0);
 	/* prefer kill(-pgid, sig) but use pid if pgid is unavailable */
-	pid_t killpid = peerpgid ? -peerpgid : peerpid;
+	pid_t killpid = (peerpgid > 1) ? -peerpgid : peerpid;
 	if (kill(killpid, sig) < 0)
 		abortmsgerrno("cannot kill %d", killpid);
 	debugmsg("forward signal %d to %d", sig, killpid);


More information about the Mercurial-devel mailing list