Simplify get_output()

src/nnn.c

@@ -837,7 +837,7 @@ static void move_cursor(int target, int ignore_scrolloff);
 static char *load_input(int fd, const char *path);
 static int set_sort_flags(int r);
 static void statusbar(char *path);
-static bool get_output(char *file, char *arg1, char *arg2, int fdout, bool multi, bool page);
+static bool get_output(char *file, char *arg1, char *arg2, int fdout, bool page);
 #ifndef NOFIFO
 static void notify_fifo(bool force);
 #endif
@@ -3188,7 +3188,7 @@ static void showfilterinfo(void)
 
 	i = getorderstr(info);
 
-	if (cfg.fileinfo && ndents && get_output("file", "-b", pdents[cur].name, -1, FALSE, FALSE))
+	if (cfg.fileinfo && ndents && get_output("file", "-b", pdents[cur].name, -1, FALSE))
 		mvaddstr(xlines - 2, 2, g_buf);
 	else {
 		snprintf(info + i, REGEX_MAX - i - 1, "  %s [/], %4s [:]",
@@ -4425,92 +4425,103 @@ static void set_smart_ctx(int ctx, char *nextpath, char **path, char *file, char
 }
 
 /*
- * Gets only a single line (that's what we need for now) or shows full command output in pager.
+ * This function does one of the following depending on the values of `fdout` and `page`:
- * Uses g_buf internally.
+ *  1) fdout == -1 && !page: Write up to CMD_LEN_MAX bytes of command output into g_buf
+ *  2) fdout == -1 && page: Create a temp file, write full command output into it and show in pager.
+ *  3) fdout != -1 && !page: Write full command output into the provided file.
+ *  4) fdout != -1 && page: Don't use! Returns FASLE.
+ *
+ * g_buf is modified only in case 1.
+ * g_tmpfpath is modified only in case 2.
  */
-static bool get_output(char *file, char *arg1, char *arg2, int fdout, bool multi, bool page)
+static bool get_output(char *file, char *arg1, char *arg2, int fdout, bool page)
 {
 	pid_t pid;
 	int pipefd[2];
 	int index = 0, flags;
 	bool ret = FALSE;
-	bool tmpfile = ((fdout == -1) && page);
+	bool have_file = fdout != -1;
-	char *argv[EXEC_ARGS_MAX] = {0};
+	int prog_in_fd = -1;
-	char *cmd = NULL;
+	int prog_out_fd = -1;
-	int fd = -1;
 	ssize_t len;
 
-	if (tmpfile) {
+	/*
+	 * In this case the logic of the function dictates that we should write the output of the command
+	 * to `fd` and show it in the pager. But since we didn't open the file descriptor we have no right
+	 * to close it, the caller must do it. We don't even know the path to pass to the pager and
+	 * it's a real hassle to get it. In general this just invites problems so we are blocking it.
+	 */
+	if (have_file && page)
+		return FALSE;
+
+	/* Setup file descriptors for child command */
+	if (!have_file && page) {
+		// Case 2
 		fdout = create_tmp_file();
 		if (fdout == -1)
 			return FALSE;
-	}
 
-	if (multi) {
+		prog_in_fd = STDIN_FILENO;
-		cmd = parseargs(file, argv, &index);
+		prog_out_fd = fdout;
-		if (!cmd)
+	} else if (have_file) {
-			return FALSE;
+		// Case 3
-	} else
+		prog_in_fd = STDIN_FILENO;
-		argv[index++] = file;
+		prog_out_fd = fdout;
+	} else {
+		// Case 1
+		if (pipe(pipefd) == -1)
+			errexit();
 
-	argv[index] = arg1;
+		for (index = 0; index < 2; ++index) {
-	argv[++index] = arg2;
+			/* Get previous flags */
+			flags = fcntl(pipefd[index], F_GETFL, 0);
 
-	if (pipe(pipefd) == -1) {
+			/* Set bit for non-blocking flag */
-		free(cmd);
+			flags |= O_NONBLOCK;
-		errexit();
-	}
 
-	for (index = 0; index < 2; ++index) {
+			/* Change flags on fd */
-		/* Get previous flags */
+			fcntl(pipefd[index], F_SETFL, flags);
-		flags = fcntl(pipefd[index], F_GETFL, 0);
+		}
 
-		/* Set bit for non-blocking flag */
+		prog_in_fd = pipefd[0];
-		flags |= O_NONBLOCK;
+		prog_out_fd = pipefd[1];
-
-		/* Change flags on fd */
-		fcntl(pipefd[index], F_SETFL, flags);
 	}
 
 	pid = fork();
 	if (pid == 0) {
-		/* In child */
+		close(prog_in_fd);
-		close(pipefd[0]);
+		dup2(prog_out_fd, STDOUT_FILENO);
-		dup2(pipefd[1], STDOUT_FILENO);
+		dup2(prog_out_fd, STDERR_FILENO);
-		dup2(pipefd[1], STDERR_FILENO);
+		close(prog_out_fd);
-		close(pipefd[1]);
+
-		execvp(*argv, argv);
+		spawn(utils[UTIL_SH_EXEC], file, arg1, arg2, F_MULTI);
 		_exit(EXIT_SUCCESS);
 	}
 
 	/* In parent */
 	waitpid(pid, NULL, 0);
-	close(pipefd[1]);
-	free(cmd);
 
-	while ((len = read(pipefd[0], g_buf, CMD_LEN_MAX - 1)) > 0) {
+	/* Do what each case should do */
+	if (!have_file && page) {
+		// Case 2
+		close(fdout);
+
+		spawn(pager, g_tmpfpath, NULL, NULL, F_CLI | F_TTY);
+
+		unlink(g_tmpfpath);
+		return TRUE;
+	} else if (have_file)
+		// Case 3
+		return TRUE;
+
+	// Case 1
+	len = read(pipefd[0], g_buf, CMD_LEN_MAX - 1);
+	if (len > 0)
 		ret = TRUE;
-		if (fdout == -1) /* Read only the first line of output to buffer */
-			break;
-		if (write(fdout, g_buf, len) != len)
-			break;
-	}
 
 	close(pipefd[0]);
-	if (!page)
+	close(pipefd[1]);
-		return ret;
+	return ret;
-
-	if (tmpfile) {
-		close(fdout);
-		close(fd);
-	}
-
-	spawn(pager, g_tmpfpath, NULL, NULL, F_CLI | F_TTY);
-
-	if (tmpfile)
-		unlink(g_tmpfpath);
-
-	return TRUE;
 }
 
 /*
@@ -4536,7 +4547,7 @@ static bool show_stats(char *fpath)
 		return FALSE;
 
 	while (r)
-		get_output(cmds[--r], fpath, NULL, fd, TRUE, FALSE);
+		get_output(cmds[--r], fpath, NULL, fd, FALSE);
 
 	close(fd);
 
@@ -4683,7 +4694,7 @@ static bool handle_archive(char *fpath /* in-out param */, char op)
 	if (op == 'x') /* extract */
 		spawn(util, arg, fpath, NULL, F_NORMAL | F_MULTI);
 	else /* list */
-		get_output(util, arg, fpath, -1, TRUE, TRUE);
+		get_output(util, arg, fpath, -1, TRUE);
 
 	if (x_to) {
 		if (chdir(xdirname(fpath)) == -1) {
@@ -5083,7 +5094,7 @@ static void show_help(const char *path)
 
 	char *prog = xgetenv(env_cfg[NNN_HELP], NULL);
 	if (prog)
-		get_output(prog, NULL, NULL, fd, TRUE, FALSE);
+		get_output(prog, NULL, NULL, fd, FALSE);
 
 	start = end = helpstr;
 	while (*end) {
@@ -5179,7 +5190,7 @@ static void run_cmd_as_plugin(const char *file, char *runfile, uchar_t flags)
 			runfile = NULL;
 
 		if (flags & F_PAGE)
-			get_output(g_buf, runfile, NULL, -1, TRUE, TRUE);
+			get_output(g_buf, runfile, NULL, -1, TRUE);
 		else // F_NOTRACE
 			spawn(g_buf, runfile, NULL, NULL, flags);
 	} else
@@ -6325,7 +6336,7 @@ static void statusbar(char *path)
 
 	attron(COLOR_PAIR(cfg.curctx + 1));
 
-	if (cfg.fileinfo && get_output("file", "-b", pdents[cur].name, -1, FALSE, FALSE))
+	if (cfg.fileinfo && get_output("file", "-b", pdents[cur].name, -1, FALSE))
 		mvaddstr(xlines - 2, 2, g_buf);
 
 	tolastln();
@@ -7045,7 +7056,7 @@ nochange:
 
 			if (cfg.useeditor
 #ifdef FILE_MIME_OPTS
-			    && get_output("file", FILE_MIME_OPTS, newpath, -1, FALSE, FALSE)
+			    && get_output("file", FILE_MIME_OPTS, newpath, -1, FALSE)
 			    && is_prefix(g_buf, "text/", 5)
 #else
 			    /* no MIME option; guess from description instead */