Submitter | Jun Wu |
---|---|
Date | July 17, 2016, 10:39 p.m. |
Message ID | <1468794496-sup-9542@x1c> |
Download | mbox | patch |
Permalink | /patch/15928/ |
State | Not Applicable |
Headers | show |
Comments
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);
Patch
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; struct sigaction sa; memset(&sa, 0, sizeof(sa));