Add reuseable abstraction over fork/exec/dup2 (#211)

main.c

@@ -240,29 +240,22 @@ void close_info(void)
 
 void open_info(void)
 {
-	int pfd[2];
+	spawn_t pfd;
 	char w[12], h[12];
+	char *argv[5];
 
 	if (info.f.err || info.fd >= 0 || win.bar.h == 0)
 		return;
 	win.bar.l.buf[0] = '\0';
-	if (pipe(pfd) < 0)
+	snprintf(w, sizeof(w), "%d", img.w);
-		return;
+	snprintf(h, sizeof(h), "%d", img.h);
-	if ((info.pid = fork()) == 0) {
+	construct_argv(argv, ARRLEN(argv), info.f.cmd, files[fileidx].name, w, h, NULL);
-		close(pfd[0]);
+	pfd = spawn(info.f.cmd, argv, X_READ);
-		dup2(pfd[1], 1);
+	if (pfd.readfd >= 0) {
-		snprintf(w, sizeof(w), "%d", img.w);
+		fcntl(pfd.readfd, F_SETFL, O_NONBLOCK);
-		snprintf(h, sizeof(h), "%d", img.h);
+		info.fd = pfd.readfd;
-		execl(info.f.cmd, info.f.cmd, files[fileidx].name, w, h, NULL);
-		error(EXIT_FAILURE, errno, "exec: %s", info.f.cmd);
-	}
-	close(pfd[1]);
-	if (info.pid < 0) {
-		close(pfd[0]);
-	} else {
-		fcntl(pfd[0], F_SETFL, O_NONBLOCK);
-		info.fd = pfd[0];
 		info.i = info.lastsep = 0;
+		info.pid = pfd.pid;
 	}
 }
 
@@ -510,15 +503,16 @@ void handle_key_handler(bool init)
 
 static bool run_key_handler(const char *key, unsigned int mask)
 {
-	pid_t pid;
 	FILE *pfs;
 	bool marked = mode == MODE_THUMB && markcnt > 0;
 	bool changed = false;
-	int f, i, pfd[2];
+	int f, i;
 	int fcnt = marked ? markcnt : 1;
 	char kstr[32];
 	struct stat *oldst, st;
 	XEvent dump;
+	char *argv[3];
+	spawn_t pfd;
 
 	if (keyhandler.f.err) {
 		if (!keyhandler.warned) {
@@ -530,41 +524,27 @@ static bool run_key_handler(const char *key, unsigned int mask)
 	if (key == NULL)
 		return false;
 
-	if (pipe(pfd) < 0) {
-		error(0, errno, "pipe");
-		return false;
-	}
-	if ((pfs = fdopen(pfd[1], "w")) == NULL) {
-		error(0, errno, "open pipe");
-		close(pfd[0]), close(pfd[1]);
-		return false;
-	}
-	oldst = emalloc(fcnt * sizeof(*oldst));
-
 	close_info();
 	strncpy(win.bar.l.buf, "Running key handler...", win.bar.l.size);
 	win_draw(&win);
 	win_set_cursor(&win, CURSOR_WATCH);
+	setenv("NSXIV_USING_NULL", options->using_null ? "1" : "0", 1);
 
 	snprintf(kstr, sizeof(kstr), "%s%s%s%s",
 	         mask & ControlMask ? "C-" : "",
 	         mask & Mod1Mask    ? "M-" : "",
 	         mask & ShiftMask   ? "S-" : "", key);
-	setenv("NSXIV_USING_NULL", options->using_null ? "1" : "0", 1);
+	construct_argv(argv, ARRLEN(argv), keyhandler.f.cmd, kstr, NULL);
-
+	pfd = spawn(keyhandler.f.cmd, argv, X_WRITE);
-	if ((pid = fork()) == 0) {
+	if (pfd.writefd < 0)
-		close(pfd[1]);
+		return false;
-		dup2(pfd[0], 0);
+	if ((pfs = fdopen(pfd.writefd, "w")) == NULL) {
-		execl(keyhandler.f.cmd, keyhandler.f.cmd, kstr, NULL);
+		close(pfd.writefd);
-		error(EXIT_FAILURE, errno, "exec: %s", keyhandler.f.cmd);
+		error(0, errno, "open pipe");
-	}
+		return false;
-	close(pfd[0]);
-	if (pid < 0) {
-		error(0, errno, "fork");
-		fclose(pfs);
-		goto end;
 	}
 
+	oldst = emalloc(fcnt * sizeof(*oldst));
 	for (f = i = 0; f < fcnt; i++) {
 		if ((marked && (files[i].flags & FF_MARK)) || (!marked && i == fileidx)) {
 			stat(files[i].path, &oldst[f]);
@@ -573,7 +553,7 @@ static bool run_key_handler(const char *key, unsigned int mask)
 		}
 	}
 	fclose(pfs);
-	while (waitpid(pid, NULL, 0) == -1 && errno == EINTR);
+	while (waitpid(pfd.pid, NULL, 0) == -1 && errno == EINTR);
 
 	for (f = i = 0; f < fcnt; i++) {
 		if ((marked && (files[i].flags & FF_MARK)) || (!marked && i == fileidx)) {
@@ -592,7 +572,6 @@ static bool run_key_handler(const char *key, unsigned int mask)
 	/* drop user input events that occurred while running the key handler */
 	while (XCheckIfEvent(win.env.dpy, &dump, is_input_ev, NULL));
 
-end:
 	if (mode == MODE_IMAGE) {
 		if (changed) {
 			img_close(&img, true);

nsxiv.h

@@ -342,6 +342,17 @@ typedef struct {
 	int stlen;
 } r_dir_t;
 
+typedef struct {
+	int readfd;
+	int writefd;
+	pid_t pid;
+} spawn_t;
+
+enum {
+	X_READ  = (1 << 0),
+	X_WRITE = (1 << 1)
+};
+
 extern const char *progname;
 
 void* emalloc(size_t);
@@ -353,6 +364,8 @@ int r_opendir(r_dir_t*, const char*, bool);
 int r_closedir(r_dir_t*);
 char* r_readdir(r_dir_t*, bool);
 int r_mkdir(char*);
+void construct_argv(char**, unsigned int, ...);
+spawn_t spawn(const char*, char *const [], unsigned int);
 
 
 /* window.c */

util.c

@@ -211,3 +211,79 @@ int r_mkdir(char *path)
 	}
 	return 0;
 }
+
+void construct_argv(char **argv, unsigned int len, ...)
+{
+	unsigned int i;
+	va_list args;
+
+	va_start(args, len);
+	for (i = 0; i < len; ++i)
+		argv[i] = va_arg(args, char *);
+	va_end(args);
+	if (argv[len-1] != NULL)
+		error(EXIT_FAILURE, 0, "argv not NULL terminated");
+}
+
+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 };
+	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) {
+		error(0, errno, "pipe: %s", cmd);
+		return status;
+	}
+
+	if (w && pipe(pfd_write) < 0) {
+		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 (r) {
+			close(pfd_read[0]);
+			close(pfd_read[1]);
+		}
+		if (w) {
+			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) {
+		if (r)
+			close(pfd_read[0]);
+		if (w)
+			close(pfd_write[1]);
+		error(0, errno, "fork: %s", cmd);
+		return status;
+	}
+
+	status.pid = pid;
+	status.readfd = pfd_read[0];
+	status.writefd = pfd_write[1];
+	return status;
+}