spawn(): improve performance and simplify API

posix_spawn() is designed especially for this purpose, and thus it's
much more lightweight and efficient than manually fork/dup/exec-ing.

on my system, it improves the performance of spawn() by about 10x. given
that we make frequent calls to potentially multiple scripts, the
increased efficiency will add up overtime.

using posix_spawn() also simplifies the logic quite a bit, despite the
very verbose function names. however it does make cleanup a bit more
complicated.

this patch uses the linux kernel style cleanup strategy [0] (which I'm
personally not a huge fan of, but it fits this situation quite nicely)
with a "stack-like" unwinding via `goto`-s.

additionally simplify the spawn() API by taking in {read,write}fd
pointers and returning the pid instead of using some custom struct.

this coincidently also fixes #299

[0]: https://www.kernel.org/doc/html/v4.10/process/coding-style.html?highlight=goto#centralized-exiting-of-functions

1 files changed, 53 insertions, 46 deletions

diff --git a/util.c b/util.c
index 9172e85..043c5bd 100644
--- a/util.c
+++ b/util.c
@@ -21,6 +21,7 @@
 
 #include <assert.h>
 #include <errno.h>
+#include <spawn.h>
 #include <stdarg.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -28,6 +29,7 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+extern char **environ;
 const char *progname = "nsxiv";
 
 void* emalloc(size_t size)
@@ -228,60 +230,65 @@ void construct_argv(char **argv, unsigned int len, ...)
 		error(EXIT_FAILURE, 0, "argv not NULL terminated");
 }
 
-spawn_t spawn(const char *cmd, char *const argv[], unsigned int flags)
+static int mkspawn_pipe(posix_spawn_file_actions_t *fa, const char *cmd, int *pfd, int dupidx)
 {
-	pid_t pid;
-	spawn_t status = { -1, -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;
-
-	if (cmd == NULL || argv == NULL || flags == 0)
-		return status;
-
-	if (r && pipe(pfd_read) < 0) {
+	int err;
+	if (pipe(pfd) < 0) {
 		error(0, errno, "pipe: %s", cmd);
-		return status;
+		return -1;
 	}
+	err = posix_spawn_file_actions_adddup2(fa, pfd[dupidx], dupidx);
+	err = err ? err : posix_spawn_file_actions_addclose(fa, pfd[0]);
+	err = err ? err : posix_spawn_file_actions_addclose(fa, pfd[1]);
+	if (err) {
+		error(0, err, "posix_spawn_file_actions: %s", cmd);
+		close(pfd[0]);
+		close(pfd[1]);
+	}
+	return err ? -1 : 0;
+}
 
-	if (w && pipe(pfd_write) < 0) {
-		error(0, errno, "pipe: %s", cmd);
-		if (r) {
-			close(pfd_read[0]);
-			close(pfd_read[1]);
-		}
-		return status;
+pid_t spawn(int *readfd, int *writefd, char *const argv[])
+{
+	pid_t pid = -1;
+	const char *cmd;
+	int err, pfd_read[2], pfd_write[2];
+	posix_spawn_file_actions_t fa;
+
+	assert(argv != NULL && argv[0] != NULL);
+	cmd = argv[0];
+
+	if ((err = posix_spawn_file_actions_init(&fa)) != 0) {
+		error(0, err, "spawn: %s", cmd);
+		return pid;
 	}
 
-	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 (readfd != NULL && mkspawn_pipe(&fa, cmd, pfd_read, 1) < 0)
+		goto err_destroy_fa;
+	if (writefd != NULL && mkspawn_pipe(&fa, cmd, pfd_write, 0) < 0)
+		goto err_close_readfd;
+
+	if ((err = posix_spawn(&pid, cmd, &fa, NULL, argv, environ)) != 0) {
+		error(0, err, "spawn: %s", cmd);
+	} else {
+		if (readfd != NULL)
+			*readfd = pfd_read[0];
+		if (writefd != NULL)
+			*writefd = pfd_write[1];
+	}
 
-		if (r) {
-			close(pfd_read[0]);
-			close(pfd_read[1]);
-		}
-		if (w) {
-			close(pfd_write[0]);
-			close(pfd_write[1]);
-		}
-		execv(cmd, argv);
-		error(EXIT_FAILURE, errno, "exec: %s", cmd);
-	} else if (pid < 0) { /* fork failed */
-		error(0, errno, "fork: %s", cmd);
-		if (r)
-			close(pfd_read[0]);
-		if (w)
+	if (writefd != NULL) {
+		close(pfd_write[0]);
+		if (pid < 0)
 			close(pfd_write[1]);
-	} else { /* in parent */
-		status.pid = pid;
-		status.readfd = pfd_read[0];
-		status.writefd = pfd_write[1];
 	}
-
-	if (r)
+err_close_readfd:
+	if (readfd != NULL) {
+		if (pid < 0)
+			close(pfd_read[0]);
 		close(pfd_read[1]);
-	if (w)
-		close(pfd_write[0]);
-	return status;
+	}
+err_destroy_fa:
+	posix_spawn_file_actions_destroy(&fa);
+	return pid;
 }