gui: Also display command's stderr output.
authorAndre Noll <maan@systemlinux.org>
Tue, 13 Mar 2012 21:17:18 +0000 (22:17 +0100)
committerAndre Noll <maan@systemlinux.org>
Sun, 18 Mar 2012 20:10:09 +0000 (21:10 +0100)
Currently, both para_cmd() and display_cmd() redirect stderr to
/dev/null, so any error messages of the command being executed do
not make it to the bottom window. This is inconvenient at times,
and makes debugging hard.

This patch prints the stderr output in the bottom window using
COLOR_ERRMSG.

As for the implementation, the global command_pipe integer
variable becomes an integer array of length two which contains
the two file descriptors for stdout and stderr. We also need two
buffers in do_select(), so command_buf becomes a two-dimensional
array. do_select() now has to monitor two file descriptors per command
and it returns only after the peer has closed both fds.

gui.c

diff --git a/gui.c b/gui.c
index 2ed66be..65b2ac0 100644 (file)
--- a/gui.c
+++ b/gui.c
@@ -53,7 +53,7 @@ static unsigned scroll_position;
 static int cmd_died, curses_active;
 static pid_t cmd_pid;
 
-static int command_pipe = -1;
+static int command_fds[2];
 static int stat_pipe = -1;
 static struct gui_args_info conf;
 
@@ -485,11 +485,12 @@ __printf_2_3 static void outputf(int color, const char* fmt,...)
        wrefresh(bot.win);
 }
 
-static int add_output_line(char *line, __a_unused void *data)
+static int add_output_line(char *line, void *data)
 {
+       int color = *(int *)data? COLOR_ERRMSG : COLOR_OUTPUT;
        if (!curses_active)
                return 1;
-       rb_add_entry(COLOR_OUTPUT, para_strdup(line));
+       rb_add_entry(color, para_strdup(line));
        return 1;
 }
 
@@ -950,15 +951,16 @@ static int open_stat_pipe(void)
        return ret;
 }
 
+#define COMMAND_BUF_SIZE 4096
+
 /*
  * This is the core select loop. Besides the (internal) signal
  * pipe, the following other fds are checked according to the mode:
  *
  * GETCH_MODE: check stdin, return when key is pressed
  *
- * COMMAND_MODE: check command_pipe and stdin. Return when a screen full
- * of output has been read or when other end has closed command pipe or
- * when any key is pressed.
+ * COMMAND_MODE: check command fds and stdin. Return when peer has closed both
+ * stdout and stderr or when any key is pressed.
  *
  * EXTERNAL_MODE: Check only signal pipe. Used when an external command
  * is running. During that time curses is disabled.  Returns when
@@ -967,11 +969,11 @@ static int open_stat_pipe(void)
 static int do_select(int mode)
 {
        fd_set rfds;
-       int ret;
-       int max_fileno;
-       char command_buf[4096] = "";
-       int cbo = 0; /* command buf offset */
+       int ret, i, max_fileno;
+       char command_buf[2][COMMAND_BUF_SIZE] = {"", ""};
+       int cbo[2] = {0, 0}; /* command buf offsets */
        struct timeval tv;
+
 repeat:
        tv.tv_sec = conf.timeout_arg  / 1000;
        tv.tv_usec = (conf.timeout_arg % 1000) * 1000;
@@ -985,8 +987,12 @@ repeat:
        /* signal pipe */
        para_fd_set(signal_pipe, &rfds, &max_fileno);
        /* command pipe only for COMMAND_MODE */
-       if (command_pipe >= 0 && mode == COMMAND_MODE)
-               para_fd_set(command_pipe, &rfds, &max_fileno);
+       if (mode == COMMAND_MODE) {
+               if (command_fds[0] >= 0)
+                       para_fd_set(command_fds[0], &rfds, &max_fileno);
+               if (command_fds[1] >= 0)
+                       para_fd_set(command_fds[1], &rfds, &max_fileno);
+       }
        ret = para_select(max_fileno + 1, &rfds, NULL, &tv);
        if (ret <= 0)
                goto check_return; /* skip fd checks */
@@ -995,21 +1001,28 @@ repeat:
        if (ret > 0)
                handle_signal(ret);
        /* read command pipe if ready */
-       if (command_pipe >= 0 && mode == COMMAND_MODE) {
-               size_t sz;
-               ret = read_nonblock(command_pipe, command_buf + cbo,
-                       sizeof(command_buf) - 1 - cbo, &rfds, &sz);
-               cbo += sz;
-               sz = cbo;
-               cbo = for_each_line(command_buf, cbo, &add_output_line, NULL);
-               if (sz != cbo)
-                       wrefresh(bot.win);
-               if (ret < 0) {
-                       PARA_NOTICE_LOG("closing command pipe: %s",
-                               para_strerror(-ret));
-                       close(command_pipe);
-                       command_pipe = -1;
-                       return 0;
+       if (mode == COMMAND_MODE) {
+               for (i = 0; i < 2; i++) {
+                       size_t sz;
+                       if (command_fds[i] < 0)
+                               continue;
+                       ret = read_nonblock(command_fds[i],
+                               command_buf[i] + cbo[i],
+                               COMMAND_BUF_SIZE - 1 - cbo[i], &rfds, &sz);
+                       cbo[i] += sz;
+                       sz = cbo[i];
+                       cbo[i] = for_each_line(command_buf[i], cbo[i],
+                               add_output_line, &i);
+                       if (sz != cbo[i])
+                               wrefresh(bot.win);
+                       if (ret < 0) {
+                               PARA_NOTICE_LOG("closing command fd %d: %s",
+                                       i, para_strerror(-ret));
+                               close(command_fds[i]);
+                               command_fds[i] = -1;
+                               if (command_fds[!i] < 0) /* both fds closed */
+                                       return 0;
+                       }
                }
        }
        ret = read_stat_pipe(&rfds);
@@ -1028,9 +1041,13 @@ check_return:
        case COMMAND_MODE:
                ret = wgetch(top.win);
                if (ret != ERR && ret != KEY_RESIZE) {
-                       if (command_pipe) {
-                               close(command_pipe);
-                               command_pipe = -1;
+                       if (command_fds[0] >= 0) {
+                               close(command_fds[0]);
+                               command_fds[0] = -1;
+                       }
+                       if (command_fds[1] >= 0) {
+                               close(command_fds[1]);
+                               command_fds[1] = -1;
                        }
                        if (cmd_pid)
                                kill(cmd_pid, SIGTERM);
@@ -1058,23 +1075,27 @@ static void send_output(void)
 {
        int ret;
 
-       if (command_pipe < 0)
-               return;
-       ret = mark_fd_nonblocking(command_pipe);
-       if (ret < 0) {
-               close(command_pipe);
-               return;
-       }
+       ret = mark_fd_nonblocking(command_fds[0]);
+       if (ret < 0)
+               goto fail;
+       ret = mark_fd_nonblocking(command_fds[1]);
+       if (ret < 0)
+               goto fail;
        if (do_select(COMMAND_MODE) >= 0)
                PARA_INFO_LOG("command complete");
        else
                PARA_NOTICE_LOG("command aborted");
        print_in_bar(COLOR_MSG, " ");
+       return;
+fail:
+       PARA_ERROR_LOG("%s\n", para_strerror(-ret));
+       close(command_fds[0]);
+       close(command_fds[1]);
 }
 
 static void para_cmd(char *cmd)
 {
-       int ret, fds[3] = {0, 1, 0};
+       int ret, fds[3] = {0, 1, 1};
        char *c = make_message(BINDIR "/para_client -- %s", cmd);
 
        outputf(COLOR_COMMAND, "%s", c);
@@ -1083,7 +1104,8 @@ static void para_cmd(char *cmd)
        free(c);
        if (ret < 0)
                return;
-       command_pipe = fds[1];
+       command_fds[0] = fds[1];
+       command_fds[1] = fds[2];
        send_output();
 }
 
@@ -1092,13 +1114,14 @@ static void para_cmd(char *cmd)
  */
 static void display_cmd(char *cmd)
 {
-       int fds[3] = {0, 1, 0};
+       int fds[3] = {0, 1, 1};
 
        print_in_bar(COLOR_MSG, "executing display command, hit any key to abort");
        outputf(COLOR_COMMAND, "%s", cmd);
        if (para_exec_cmdline_pid(&cmd_pid, cmd, fds) < 0)
                return;
-       command_pipe = fds[1];
+       command_fds[0] = fds[1];
+       command_fds[1] = fds[2];
        send_output();
 }