[master] 918cd81 Don't start a subprocess just to kill our own child process.

[Dridi Boukelmoune]

On Wed, Nov 16, 2016 at 1:32 PM, Poul-Henning Kamp <phk at freebsd.org> wrote:
>
> commit 918cd814e343dfe99aa949524f2a4b3647133024
> Author: Poul-Henning Kamp <phk at FreeBSD.org>
> Date:   Wed Nov 16 12:31:01 2016 +0000
>
>     Don't start a subprocess just to kill our own child process.
>
> diff --git a/bin/varnishtest/vtc_process.c b/bin/varnishtest/vtc_process.c
> index 3dba551..dd553c9 100644
> --- a/bin/varnishtest/vtc_process.c
> +++ b/bin/varnishtest/vtc_process.c
> @@ -247,8 +247,7 @@ process_run(struct process *p)
>  static void
>  process_kill(const struct process *p, const char *sig)
>  {
> -       int s;
> -       char buf[64];
> +       int j;
>
>         CHECK_OBJ_NOTNULL(p, PROCESS_MAGIC);
>         AN(sig);
> @@ -256,12 +255,19 @@ process_kill(const struct process *p, const char *sig)
>         if (!p->running || !p->pid)
>                 vtc_log(p->vl, 0, "Cannot signal a non-running process");
>
> -       bprintf(buf, "kill -%s -%d", sig, p->pid);
> -       vtc_log(p->vl, 4, "CMD: %s", buf);
> -
> -       s = system(buf);
> -       if (s != 0)
> -               vtc_log(p->vl, 0, "Failed to send signal (exit status: %d)", s);
> +       if (!strcmp(sig, "TERM"))
> +               j = SIGTERM;
> +       else if (!strcmp(sig, "HUP"))
> +               j = SIGHUP;
> +       else if (!strcmp(sig, "KILL"))
> +               j = SIGKILL;
> +       else
> +               j = strtoul(sig, NULL, 10);

Hi,

This had been discussed during the review of background processes in
varnishtest. The consensus was to let the kill(1) command do the
signal parsing for better portability.

Since this patch is restricting the signal spec to 3 strings or a
number, either revert this change or update the documentation too.

Cheers,
Dridi

> +
> +       if (kill(p->pid, j) < 0)
> +               vtc_log(p->vl, 0, "Failed to send signal %d (%s)", j, strerror(errno));
> +       else
> +               vtc_log(p->vl, 4, "Sent signal %d", j);
>  }
>
>  static inline void
>
> _______________________________________________
> varnish-commit mailing list
> varnish-commit at varnish-cache.org
> https://www.varnish-cache.org/lists/mailman/listinfo/varnish-commit