From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <alexander.turenko@tarantool.org>
Received: from smtp49.i.mail.ru (smtp49.i.mail.ru [94.100.177.109])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id 2F08A4696C3
 for <tarantool-patches@dev.tarantool.org>;
 Sun, 12 Apr 2020 04:28:00 +0300 (MSK)
Date: Sun, 12 Apr 2020 04:27:58 +0300
From: Alexander Turenko <alexander.turenko@tarantool.org>
Message-ID: <20200412012758.d4pk7qia7pqjp2vr@tkn_work_nb>
References: <20200410144021.5704-1-gorcunov@gmail.com>
 <20200410144021.5704-2-gorcunov@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <20200410144021.5704-2-gorcunov@gmail.com>
Subject: Re: [Tarantool-patches] [PATCH 1/2] popen: Allow to kill process
	group
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: tml <tarantool-patches@dev.tarantool.org>

> +	/*
> +	 * Killing group of signals allowed for a new
> +	 * session only where it makes sense, otherwise
> +	 * child gonna inherit group and we will be killing
> +	 * ourself.
> +	 */
> +	if (opts->flags & POPEN_FLAG_GROUP_SIGNAL &&
> +	    (opts->flags & POPEN_FLAG_SETSID) == 0) {
> +		diag_set(IllegalParams,
> +			 "popen: group signal without setting sid");
> +		return NULL;
> +	}
> +

Since the patchset with the preliminary patches is land to master and it
contains formal list of possible errors for popen_new(), I propose the
following addition:

diff --git a/src/lib/core/popen.c b/src/lib/core/popen.c
index 593e72d4d..fcc562734 100644
--- a/src/lib/core/popen.c
+++ b/src/lib/core/popen.c
@@ -812,6 +812,8 @@ signal_reset(void)
  *
  * Possible errors:
  *
+ * - IllegalParams: a parameter check fails:
+ *   - group signal is set, while setsid is not.
  * - SystemError: dup(), fcntl(), pipe(), vfork() or close() fails
  *   in the parent process.
  * - SystemError: (temporary restriction) one of std{in,out,err}