]> Sergey Matveev's repositories - nnn.git/commitdiff
Simplify get_output()
authorKlzXS <klzx+github@klzx.cf>
Sat, 14 Jan 2023 21:11:10 +0000 (22:11 +0100)
committerKlzXS <klzx+github@klzx.cf>
Sat, 14 Jan 2023 21:11:10 +0000 (22:11 +0100)
src/nnn.c

index a5a8d770bb6a204cd9c02efad2cdb61ddca0db30..2cba953b86b526aebe71583f8ea2f3705cb59107 100644 (file)
--- a/src/nnn.c
+++ b/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.
- * Uses g_buf internally.
+ * This function does one of the following depending on the values of `fdout` and `page`:
+ *  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);
-       char *argv[EXEC_ARGS_MAX] = {0};
-       char *cmd = NULL;
-       int fd = -1;
+       bool have_file = fdout != -1;
+       int prog_in_fd = -1;
+       int prog_out_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) {
-               cmd = parseargs(file, argv, &index);
-               if (!cmd)
-                       return FALSE;
-       } else
-               argv[index++] = file;
 
-       argv[index] = arg1;
-       argv[++index] = arg2;
+               prog_in_fd = STDIN_FILENO;
+               prog_out_fd = fdout;
+       } else if (have_file) {
+               // Case 3
+               prog_in_fd = STDIN_FILENO;
+               prog_out_fd = fdout;
+       } else {
+               // Case 1
+               if (pipe(pipefd) == -1)
+                       errexit();
 
-       if (pipe(pipefd) == -1) {
-               free(cmd);
-               errexit();
-       }
+               for (index = 0; index < 2; ++index) {
+                       /* Get previous flags */
+                       flags = fcntl(pipefd[index], F_GETFL, 0);
 
-       for (index = 0; index < 2; ++index) {
-               /* Get previous flags */
-               flags = fcntl(pipefd[index], F_GETFL, 0);
+                       /* Set bit for non-blocking flag */
+                       flags |= O_NONBLOCK;
 
-               /* Set bit for non-blocking flag */
-               flags |= O_NONBLOCK;
+                       /* Change flags on fd */
+                       fcntl(pipefd[index], F_SETFL, flags);
+               }
 
-               /* Change flags on fd */
-               fcntl(pipefd[index], F_SETFL, flags);
+               prog_in_fd = pipefd[0];
+               prog_out_fd = pipefd[1];
        }
 
        pid = fork();
        if (pid == 0) {
-               /* In child */
-               close(pipefd[0]);
-               dup2(pipefd[1], STDOUT_FILENO);
-               dup2(pipefd[1], STDERR_FILENO);
-               close(pipefd[1]);
-               execvp(*argv, argv);
+               close(prog_in_fd);
+               dup2(prog_out_fd, STDOUT_FILENO);
+               dup2(prog_out_fd, STDERR_FILENO);
+               close(prog_out_fd);
+
+               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) {
-               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)
-               return ret;
-
-       if (tmpfile) {
+       /* Do what each case should do */
+       if (!have_file && page) {
+               // Case 2
                close(fdout);
-               close(fd);
-       }
 
-       spawn(pager, g_tmpfpath, NULL, NULL, F_CLI | F_TTY);
+               spawn(pager, g_tmpfpath, NULL, NULL, F_CLI | F_TTY);
 
-       if (tmpfile)
                unlink(g_tmpfpath);
+               return TRUE;
+       } else if (have_file)
+               // Case 3
+               return TRUE;
 
-       return TRUE;
+       // Case 1
+       len = read(pipefd[0], g_buf, CMD_LEN_MAX - 1);
+       if (len > 0)
+               ret = TRUE;
+
+       close(pipefd[0]);
+       close(pipefd[1]);
+       return ret;
 }
 
 /*
@@ -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 */