From 6c23fdfd5fa1c394b71b7e26f5d921845c724ee4 Mon Sep 17 00:00:00 2001 From: KlzXS Date: Sat, 14 Jan 2023 22:11:10 +0100 Subject: [PATCH 1/4] Simplify get_output() --- src/nnn.c | 137 +++++++++++++++++++++++++++++------------------------- 1 file changed, 74 insertions(+), 63 deletions(-) diff --git a/src/nnn.c b/src/nnn.c index a5a8d770b..2cba953b8 100644 --- 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 @@ static bool browse(char *ipath, const char *session, int pkey) 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 */ From 1a2f783b757699805487eb701d85ca0917692245 Mon Sep 17 00:00:00 2001 From: KlzXS Date: Sat, 14 Jan 2023 22:31:16 +0100 Subject: [PATCH 2/4] Give better names to variables --- src/nnn.c | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/nnn.c b/src/nnn.c index 2cba953b8..78cf611c8 100644 --- a/src/nnn.c +++ b/src/nnn.c @@ -4441,8 +4441,8 @@ static bool get_output(char *file, char *arg1, char *arg2, int fdout, bool page) int index = 0, flags; bool ret = FALSE; bool have_file = fdout != -1; - int prog_in_fd = -1; - int prog_out_fd = -1; + int cmd_in_fd = -1; + int cmd_out_fd = -1; ssize_t len; /* @@ -4461,12 +4461,12 @@ static bool get_output(char *file, char *arg1, char *arg2, int fdout, bool page) if (fdout == -1) return FALSE; - prog_in_fd = STDIN_FILENO; - prog_out_fd = fdout; + cmd_in_fd = STDIN_FILENO; + cmd_out_fd = fdout; } else if (have_file) { // Case 3 - prog_in_fd = STDIN_FILENO; - prog_out_fd = fdout; + cmd_in_fd = STDIN_FILENO; + cmd_out_fd = fdout; } else { // Case 1 if (pipe(pipefd) == -1) @@ -4483,16 +4483,17 @@ static bool get_output(char *file, char *arg1, char *arg2, int fdout, bool page) fcntl(pipefd[index], F_SETFL, flags); } - prog_in_fd = pipefd[0]; - prog_out_fd = pipefd[1]; + cmd_in_fd = pipefd[0]; + cmd_out_fd = pipefd[1]; } pid = fork(); if (pid == 0) { - close(prog_in_fd); - dup2(prog_out_fd, STDOUT_FILENO); - dup2(prog_out_fd, STDERR_FILENO); - close(prog_out_fd); + /* In child */ + close(cmd_in_fd); + dup2(cmd_out_fd, STDOUT_FILENO); + dup2(cmd_out_fd, STDERR_FILENO); + close(cmd_out_fd); spawn(utils[UTIL_SH_EXEC], file, arg1, arg2, F_MULTI); _exit(EXIT_SUCCESS); From 8a1e32d9eb191db2f1b75fda6f15bf6625ecd3fa Mon Sep 17 00:00:00 2001 From: KlzXS Date: Sat, 14 Jan 2023 23:18:22 +0100 Subject: [PATCH 3/4] Make CI happy --- src/nnn.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nnn.c b/src/nnn.c index 78cf611c8..4b2d0a3d6 100644 --- a/src/nnn.c +++ b/src/nnn.c @@ -4511,7 +4511,9 @@ static bool get_output(char *file, char *arg1, char *arg2, int fdout, bool page) unlink(g_tmpfpath); return TRUE; - } else if (have_file) + } + + if (have_file) // Case 3 return TRUE; From 428c652d365b8571ea8b93bfca9e505f5ff14251 Mon Sep 17 00:00:00 2001 From: Arun Date: Sun, 15 Jan 2023 10:59:36 +0530 Subject: [PATCH 4/4] Concatenate arguments to pass to `sh` Co-authored-by: KlzXS Co-authored-by: Arun Prakash Jana --- src/nnn.c | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/src/nnn.c b/src/nnn.c index 4b2d0a3d6..c70392bb4 100644 --- a/src/nnn.c +++ b/src/nnn.c @@ -4490,12 +4490,29 @@ static bool get_output(char *file, char *arg1, char *arg2, int fdout, bool page) pid = fork(); if (pid == 0) { /* In child */ + char *bufptr = file; + close(cmd_in_fd); dup2(cmd_out_fd, STDOUT_FILENO); dup2(cmd_out_fd, STDERR_FILENO); close(cmd_out_fd); - spawn(utils[UTIL_SH_EXEC], file, arg1, arg2, F_MULTI); + if (bufptr && arg1) { + char argbuf[CMD_LEN_MAX]; + + len = xstrsncpy(argbuf, file, xstrlen(file) + 1); + argbuf[len - 1] = ' '; + bufptr = argbuf + len; + len = xstrsncpy(bufptr, arg1, xstrlen(arg1) + 1); + if (arg2) { + bufptr[len - 1] = ' '; + xstrsncpy(bufptr + len, arg2, xstrlen(arg2) + 1); + } + + bufptr = argbuf; + } + + spawn(utils[UTIL_SH_EXEC], bufptr, NULL, NULL, F_MULTI); _exit(EXIT_SUCCESS); }