fix: potentially printing wrong error message (#321)

it's possible for the close() calls to override the errno resulting in
incorrect error message being printed.

call error() immediately to avoid such possibilities.

also refactor a couple conditions to avoid doing multiple checks.

Reviewed-on: https://codeberg.org/nsxiv/nsxiv/pulls/321
Reviewed-by: Berke Kocaoğlu <berke.kocaoglu@metu.edu.tr>

1 files changed, 16 insertions, 21 deletions

diff --git a/util.c b/util.c
index e9c899b..c207e60 100644
--- a/util.c
+++ b/util.c
@@ -233,8 +233,7 @@ spawn_t spawn(const char *cmd, char *const argv[], unsigned int flags)
 {
 	pid_t pid;
 	spawn_t status = { -1, -1, -1 };
-	int pfd_read[2] = { -1, -1 };
-	int pfd_write[2] = { -1, -1 };
+	int pfd_read[2] = { -1, -1 }, pfd_write[2] = { -1, -1 };
 	const bool r = flags & X_READ;
 	const bool w = flags & X_WRITE;
 
@@ -247,16 +246,18 @@ spawn_t spawn(const char *cmd, char *const argv[], unsigned int flags)
 	}
 
 	if (w && pipe(pfd_write) < 0) {
+		error(0, errno, "pipe: %s", cmd);
 		if (r) {
 			close(pfd_read[0]);
 			close(pfd_read[1]);
 		}
-		error(0, errno, "pipe: %s", cmd);
 		return status;
 	}
 
-	if ((pid = fork()) == 0) {
-		bool err = (r && dup2(pfd_read[1], 1) < 0) || (w && dup2(pfd_write[0], 0) < 0);
+	if ((pid = fork()) == 0) { /* in child */
+		if ((r && dup2(pfd_read[1], 1) < 0) || (w && dup2(pfd_write[0], 0) < 0))
+			error(EXIT_FAILURE, errno, "dup2: %s", cmd);
+
 		if (r) {
 			close(pfd_read[0]);
 			close(pfd_read[1]);
@@ -265,29 +266,23 @@ spawn_t spawn(const char *cmd, char *const argv[], unsigned int flags)
 			close(pfd_write[0]);
 			close(pfd_write[1]);
 		}
-
-		if (err)
-			error(EXIT_FAILURE, errno, "dup2: %s", cmd);
 		execv(cmd, argv);
 		error(EXIT_FAILURE, errno, "exec: %s", cmd);
-	}
-
-	if (r)
-		close(pfd_read[1]);
-	if (w)
-		close(pfd_write[0]);
-
-	if (pid < 0) {
+	} else if (pid < 0) { /* fork failed */
+		error(0, errno, "fork: %s", cmd);
 		if (r)
 			close(pfd_read[0]);
 		if (w)
 			close(pfd_write[1]);
-		error(0, errno, "fork: %s", cmd);
-		return status;
+	} else { /* in parent */
+		status.pid = pid;
+		status.readfd = pfd_read[0];
+		status.writefd = pfd_write[1];
 	}
 
-	status.pid = pid;
-	status.readfd = pfd_read[0];
-	status.writefd = pfd_write[1];
+	if (r)
+		close(pfd_read[1]);
+	if (w)
+		close(pfd_write[0]);
 	return status;
 }