[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