[Tarantool-patches] [PATCH 10/13] popen: add missed diag_set() in popen IO functions

[Alexander Turenko]

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