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 49E4E4696CC for ; Fri, 10 Apr 2020 05:51:23 +0300 (MSK) From: Alexander Turenko Date: Fri, 10 Apr 2020 05:50:48 +0300 Message-Id: <0a37f62adb35dd6257d93a90b2176093700bcf51.1586486220.git.alexander.turenko@tarantool.org> In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH 10/13] popen: add missed diag_set() in popen IO functions List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Cyrill Gorcunov Cc: tarantool-patches@dev.tarantool.org Our usual convention for C code is to return a negative value at failure and set an entry to the diagnostics area. When code uses this convention consistently, it is much easier to handle failures when using it: you always know where to find an error type and message and how to pass the error to a C or Lua caller. See also the previous commit ('popen: add missed diag_set in popen_signal/delete'). Part of #4031 --- src/lib/core/popen.c | 100 +++++++++++++++++++++++++++++++------------ 1 file changed, 72 insertions(+), 28 deletions(-) diff --git a/src/lib/core/popen.c b/src/lib/core/popen.c index c54e0b211..bf7d597bd 100644 --- a/src/lib/core/popen.c +++ b/src/lib/core/popen.c @@ -191,22 +191,20 @@ handle_free(struct popen_handle *handle) } /** - * Test if the handle can run io operation. + * Test if the handle can run a requested IO operation. + * + * Returns 0 if so and -1 otherwise (and set a diag). */ -static inline bool +static inline int popen_may_io(struct popen_handle *handle, unsigned int io_flags) { - if (!handle) { - errno = ESRCH; - return false; - } - if (!(io_flags & handle->flags)) { - errno = EINVAL; - return false; + diag_set(IllegalParams, "popen: handle does not support the " + "requested IO operation"); + return -1; } - return true; + return 0; } /** @@ -273,6 +271,27 @@ stdX_str(unsigned int index) /** * Write data to the child stdin. + * + * Yield until all @a count bytes will be written. + * + * Returns @a count at success, otherwise returns -1 and set a + * diag. + * + * Possible errors: + * + * - IllegalParams: a parameter check fails: + * - count: data is too big. + * - flags: POPEN_FLAG_FD_STDIN bit is unset. + * - handle: handle does not support the requested IO operation. + * - SocketError: an IO error occurs at write(). + * - TimedOut: @a timeout quota is exceeded. + * - FiberIsCancelled: cancelled by an outside code. + * + * An error may occur after a partial write. There is not way to + * enquire amount of written bytes in the case. + * + * FIXME: Provide an info re amount written bytes in the case. + * Say, return -(written) in the case. */ int popen_write_timeout(struct popen_handle *handle, const void *buf, @@ -281,20 +300,21 @@ popen_write_timeout(struct popen_handle *handle, const void *buf, { assert(handle != NULL); - int idx = STDIN_FILENO; + if (count > (size_t)SSIZE_MAX) { + diag_set(IllegalParams, "popen: data is too big"); + return -1; + } if (!(flags & POPEN_FLAG_FD_STDIN)) { - errno = EINVAL; - return -1; + diag_set(IllegalParams, + "popen: POPEN_FLAG_FD_STDIN bit is unset"); + return -1; } - if (!popen_may_io(handle, flags)) + if (popen_may_io(handle, flags) != 0) return -1; - if (count > (size_t)SSIZE_MAX) { - errno = E2BIG; - return -1; - } + int idx = STDIN_FILENO; say_debug("popen: %d: write idx [%s:%d] buf %p count %zu " "fds %d timeout %.9g", @@ -307,6 +327,26 @@ popen_write_timeout(struct popen_handle *handle, const void *buf, /** * Read data from a child's peer with timeout. + * + * Yield until some data will be available for read. + * + * Returns amount of read bytes at success, otherwise returns -1 + * and set a diag. + * + * Zero return value means EOF. + * + * Note: Less then @a count bytes may be available for read at a + * moment, so a return value less then @a count does not mean EOF. + * + * Possible errors: + * + * - IllegalParams: a parameter check fails: + * - count: buffer is too big. + * - flags: POPEN_FLAG_FD_STD{OUT,ERR} are unset both. + * - handle: handle does not support the requested IO operation. + * - SocketError: an IO error occurs at read(). + * - TimedOut: @a timeout quota is exceeded. + * - FiberIsCancelled: cancelled by an outside code. */ ssize_t popen_read_timeout(struct popen_handle *handle, void *buf, @@ -315,24 +355,28 @@ popen_read_timeout(struct popen_handle *handle, void *buf, { assert(handle != NULL); - int idx = flags & POPEN_FLAG_FD_STDOUT ? - STDOUT_FILENO : STDERR_FILENO; + if (count > (size_t)SSIZE_MAX) { + diag_set(IllegalParams, "popen: buffer is too big"); + return -1; + } if (!(flags & (POPEN_FLAG_FD_STDOUT | POPEN_FLAG_FD_STDERR))) { - errno = EINVAL; - return -1; + diag_set(IllegalParams, "popen: POPEN_FLAG_FD_STD{OUT,ERR} are " + "unset both"); + return -1; } - if (!popen_may_io(handle, flags)) + if (flags & POPEN_FLAG_FD_STDOUT && flags & POPEN_FLAG_FD_STDERR) { + diag_set(IllegalParams, "popen: reading from both stdout and " + "stderr at one call is not supported"); return -1; + } - if (count > (size_t)SSIZE_MAX) { - errno = E2BIG; + if (popen_may_io(handle, flags) != 0) return -1; - } - if (timeout < 0.) - timeout = TIMEOUT_INFINITY; + int idx = flags & POPEN_FLAG_FD_STDOUT ? + STDOUT_FILENO : STDERR_FILENO; say_debug("popen: %d: read idx [%s:%d] buf %p count %zu " "fds %d timeout %.9g", -- 2.25.0