From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 35A9E4696C5 for ; Fri, 10 Apr 2020 05:51:09 +0300 (MSK) From: Alexander Turenko Date: Fri, 10 Apr 2020 05:50:41 +0300 Message-Id: In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 03/13] popen: add missed diag_set in popen_signal/delete List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tarantool-patches@dev.tarantool.org Lua API will use content of the diagnostics area to report an error to a caller, so it is critical to always have proper diagnostics at failure. Part of #4031 --- src/lib/core/popen.c | 37 +++++++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/src/lib/core/popen.c b/src/lib/core/popen.c index 3a0ac4b53..3fcbc325a 100644 --- a/src/lib/core/popen.c +++ b/src/lib/core/popen.c @@ -164,16 +164,19 @@ popen_may_io(struct popen_handle *handle, unsigned int idx, /** * Test if the handle still have a living child process. + * + * Return -1 and set errno to ESRCH when a process does not + * exist. Otherwise return 0. */ -static inline bool +static inline int popen_may_pidop(struct popen_handle *handle) { assert(handle != NULL); if (handle->pid == -1) { errno = ESRCH; - return false; + return -1; } - return true; + return 0; } /** @@ -423,6 +426,10 @@ popen_state_str(unsigned int state) /** * Send a signal to a child process. + * + * Return 0 at success or -1 at failure (and set a diag). + * + * Set errno to ESRCH when a process does not exist. */ int popen_send_signal(struct popen_handle *handle, int signo) @@ -434,16 +441,22 @@ popen_send_signal(struct popen_handle *handle, int signo) /* * A child may be killed or exited already. */ - if (!popen_may_pidop(handle)) + ret = popen_may_pidop(handle); + if (ret == 0) { + say_debug("popen: kill %d signo %d", handle->pid, signo); + assert(handle->pid != -1); + ret = kill(handle->pid, signo); + } + if (ret < 0 && errno == ESRCH) { + diag_set(SystemError, "Attempt to send a signal %d to a " + "process that does not exist anymore", signo); return -1; - - say_debug("popen: kill %d signo %d", handle->pid, signo); - ret = kill(handle->pid, signo); - if (ret < 0) { + } else if (ret < 0) { diag_set(SystemError, "Unable to kill %d signo %d", handle->pid, signo); + return -1; } - return ret; + return 0; } /** @@ -460,7 +473,11 @@ popen_delete(struct popen_handle *handle) assert(handle != NULL); - if (popen_send_signal(handle, SIGKILL) && errno != ESRCH) + /* + * Unable to kill the process -- give an error. + * The process is not exist already -- pass over. + */ + if (popen_send_signal(handle, SIGKILL) != 0 && errno != ESRCH) return -1; for (i = 0; i < lengthof(handle->ios); i++) { -- 2.25.0