Re: [Tarantool-patches] [PATCH 1/2] popen: Allow to kill process group

[Alexander Turenko <alexander.turenko@tarantool.org>]

On Fri, Apr 10, 2020 at 05:40:20PM +0300, Cyrill Gorcunov wrote:
> As Alexander pointed out this might be useful
> for running a pipe of programs inside shell
> (i.e. popen.shell('foo | bar | baz', 'r')).
> 
> Reported-by: Alexander Turenko <alexander.turenko@tarantool.org>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>

Acked-by: Alexander Turenko <alexander.turenko@tarantool.org>

> @@ -443,10 +456,17 @@ popen_send_signal(struct popen_handle *handle, int signo)
>  	if (!popen_may_pidop(handle))
>  		return -1;
>  
> -	say_debug("popen: kill %d signo %d", handle->pid, signo);
> -	ret = kill(handle->pid, signo);
> +	say_debug("popen: %s %d signo %d",
> +		  handle->flags & POPEN_FLAG_GROUP_SIGNAL ?
> +		  "killpg" : "kill", handle->pid, signo);
> +	if (handle->flags & POPEN_FLAG_GROUP_SIGNAL)
> +		ret = killpg(handle->pid, signo);
> +	else
> +		ret = kill(handle->pid, signo);
>  	if (ret < 0) {
> -		diag_set(SystemError, "Unable to kill %d signo %d",
> +		diag_set(SystemError, "Unable to %s %d signo %d",
> +			 handle->flags & POPEN_FLAG_GROUP_SIGNAL ?
> +			 "killpg" : "kill",
>  			 handle->pid, signo);
>  	}

Nit: I would define a string constant, it will be easier to read I
guess.

 | const char *kill_func = handle->flags & POPEN_FLAG_GROUP_SIGNAL ?
 | 		"killpg" : "kill";

This function is changed in the 'Popen Lua API: preliminary patches'
patchset, which is pushed to master now. Need to be rebased so.