gui: Get rid of do_select()'s mode parameter and call it only once.

The current code calls do_select() from three different locations. The
mode parameter indicates whether a command is currently running
and if it is a display command or an external command (where curses
is disabled).  This is unnecessarily complex as we can tell from the
values of the command_fd[] array and the cmd_pid variable which mode
we are in.

This commit introduces get_select_mode() which looks at these variables
and returns the current mode. This allows to get rid of the mode
parameter of do_select() and to call do_select() only once at startup
from main(). It now runs in an endless loop, checking the current mode
in each iteration. The other two call sites, exec_and_display_cmd()
and external_cmd(), don't need to call do_select() any more.

This change removes more lines than it adds and makes the logic
of para_gui much simpler to follow. It is also the last big step
to replace the do_select() loop in favor of the standard paraslash
scheduler.

diff --git a/gui.c b/gui.c
index 6eea455fdcd6175c4c4615c27b280e794be3a907..ef9410cc26dc6a26268a9873202495bd43ffaa7d 100644 (file)
--- a/gui.c
+++ b/gui.c
@@ -53,8 +53,7 @@ static int stat_pipe = -1;
 static struct gui_args_info conf;
 static int loglevel;
 
 static struct gui_args_info conf;
 static int loglevel;
 

-enum {GETCH_MODE, COMMAND_MODE, EXTERNAL_MODE};
-
+enum gui_select_mode{GETCH_MODE, COMMAND_MODE, EXTERNAL_MODE};

 
 /**
  * Codes for various colors.
 
 /**
  * Codes for various colors.


@@ -809,8 +808,11 @@ reap_next_child:
        ret = para_reap_child(&pid);
        if (ret <= 0)
                return;
        ret = para_reap_child(&pid);
        if (ret <= 0)
                return;

-       if (pid == cmd_pid)
+       if (pid == cmd_pid) {

                cmd_pid = 0;
                cmd_pid = 0;

+               init_curses();
+               print_in_bar(COLOR_MSG, " ");
+       }

        goto reap_next_child;
 }
 
        goto reap_next_child;
 }
 


@@ -979,9 +981,19 @@ success:
 
 #define COMMAND_BUF_SIZE 32768
 
 
 #define COMMAND_BUF_SIZE 32768
 

-static void command_pre_select(int mode, fd_set *rfds, int *max_fileno)
+static enum gui_select_mode get_select_mode(void)
+{
+       if (command_fds[0] >= 0 || command_fds[1] >= 0)
+               return COMMAND_MODE;
+       if (cmd_pid > 0)
+               return EXTERNAL_MODE;
+       return GETCH_MODE;
+}
+
+static void command_pre_select(fd_set *rfds, int *max_fileno)

 {
 {

-       /* check command pipe only in COMMAND_MODE */
+       enum gui_select_mode mode = get_select_mode();
+

        if (mode != COMMAND_MODE)
                return;
        if (command_fds[0] >= 0)
        if (mode != COMMAND_MODE)
                return;
        if (command_fds[0] >= 0)


@@ -990,15 +1002,16 @@ static void command_pre_select(int mode, fd_set *rfds, int *max_fileno)
                para_fd_set(command_fds[1], rfds, max_fileno);
 }
 
                para_fd_set(command_fds[1], rfds, max_fileno);
 }
 

-static int command_post_select(int mode, fd_set *rfds)
+static void command_post_select(fd_set *rfds)

 {
        int i, ret;
        static char command_buf[2][COMMAND_BUF_SIZE];
        static int cbo[2]; /* command buf offsets */
        static unsigned flags[2]; /* for for_each_line() */
 {
        int i, ret;
        static char command_buf[2][COMMAND_BUF_SIZE];
        static int cbo[2]; /* command buf offsets */
        static unsigned flags[2]; /* for for_each_line() */

+       enum gui_select_mode mode = get_select_mode();

 
        if (mode != COMMAND_MODE)
 
        if (mode != COMMAND_MODE)

-               return 0;
+               return;

        for (i = 0; i < 2; i++) {
                size_t sz;
                if (command_fds[i] < 0)
        for (i = 0; i < 2; i++) {
                size_t sz;
                if (command_fds[i] < 0)


@@ -1014,15 +1027,16 @@ static int command_post_select(int mode, fd_set *rfds)
                        wrefresh(bot.win);
                        flags[i] = 0;
                }
                        wrefresh(bot.win);
                        flags[i] = 0;
                }

-               if (ret < 0) {
-                       PARA_NOTICE_LOG("closing command fd %d: %s",
-                               i, para_strerror(-ret));
+               if (ret < 0 || cmd_pid == 0) {
+                       if (ret < 0)
+                               PARA_NOTICE_LOG("closing command fd %d: %s",
+                                       i, para_strerror(-ret));

                        close(command_fds[i]);
                        command_fds[i] = -1;
                        flags[i] = 0;
                        cbo[i] = 0;
                        if (command_fds[!i] < 0) /* both fds closed */
                        close(command_fds[i]);
                        command_fds[i] = -1;
                        flags[i] = 0;
                        cbo[i] = 0;
                        if (command_fds[!i] < 0) /* both fds closed */

-                               return -1;
+                               return;

                }
                if (cbo[i] == COMMAND_BUF_SIZE - 1) {
                        PARA_NOTICE_LOG("discarding overlong line");
                }
                if (cbo[i] == COMMAND_BUF_SIZE - 1) {
                        PARA_NOTICE_LOG("discarding overlong line");


@@ -1030,10 +1044,15 @@ static int command_post_select(int mode, fd_set *rfds)
                        flags[i] = FELF_DISCARD_FIRST;
                }
        }
                        flags[i] = FELF_DISCARD_FIRST;
                }
        }

-       return 1;

 }
 
 }
 

-static int do_select(int mode);
+static void input_pre_select(fd_set *rfds, int *max_fileno)
+{
+       enum gui_select_mode mode = get_select_mode();
+
+       if (mode == GETCH_MODE || mode == COMMAND_MODE)
+               para_fd_set(STDIN_FILENO, rfds, max_fileno);
+}

 
 /* read from command pipe and print data to bot window */
 static void exec_and_display_cmd(const char *cmd)
 
 /* read from command pipe and print data to bot window */
 static void exec_and_display_cmd(const char *cmd)


@@ -1052,11 +1071,6 @@ static void exec_and_display_cmd(const char *cmd)
                goto fail;
        command_fds[0] = fds[1];
        command_fds[1] = fds[2];
                goto fail;
        command_fds[0] = fds[1];
        command_fds[1] = fds[2];

-       if (do_select(COMMAND_MODE) >= 0)
-               PARA_INFO_LOG("command complete\n");
-       else
-               PARA_NOTICE_LOG("command aborted\n");
-       print_in_bar(COLOR_MSG, " ");

        return;
 fail:
        PARA_ERROR_LOG("%s\n", para_strerror(-ret));
        return;
 fail:
        PARA_ERROR_LOG("%s\n", para_strerror(-ret));


@@ -1090,10 +1104,7 @@ static void external_cmd(char *cmd)
        if (cmd_pid)
                return;
        shutdown_curses();
        if (cmd_pid)
                return;
        shutdown_curses();

-       if (para_exec_cmdline_pid(&cmd_pid, cmd, fds) < 0)
-               return;
-       do_select(EXTERNAL_MODE);
-       init_curses();
+       para_exec_cmdline_pid(&cmd_pid, cmd, fds);

 }
 
 static void handle_command(int c)
 }
 
 static void handle_command(int c)


@@ -1138,37 +1149,20 @@ static void handle_command(int c)
                km_keyname(c));
 }
 
                km_keyname(c));
 }
 

-static void input_pre_select(int mode, fd_set *rfds, int *max_fileno)
-{
-       if (mode == GETCH_MODE || mode == COMMAND_MODE)
-               para_fd_set(STDIN_FILENO, rfds, max_fileno);
-}
-
-static int input_post_select(int mode)
+static void input_post_select(void)

 {
        int ret;
 {
        int ret;

+       enum gui_select_mode mode = get_select_mode();

 
 

-       switch (mode) {
-       case COMMAND_MODE:
-               ret = wgetch(top.win);
-               if (ret != ERR && ret != KEY_RESIZE) {
-                       if (cmd_pid)
-                               kill(cmd_pid, SIGTERM);
-                       return -1;
-               }
-               return 0;
-       case GETCH_MODE:
-               ret = wgetch(top.win);
-               if (ret != ERR && ret != KEY_RESIZE)
-                       return ret;
-               return 0;
-       case EXTERNAL_MODE:
-               if (cmd_pid == 0)
-                       return -1;
-               return 0;
-       default:
-               assert(false); /* bug */
-       }
+       if (mode == EXTERNAL_MODE)
+               return;
+       ret = wgetch(top.win);
+       if (ret == ERR || ret == KEY_RESIZE)
+               return;
+       if (mode == GETCH_MODE)
+               return handle_command(ret);
+       if (cmd_pid != 0)
+               kill(cmd_pid, SIGTERM);

 }
 
 static void signal_pre_select(fd_set *rfds, int *max_fileno)
 }
 
 static void signal_pre_select(fd_set *rfds, int *max_fileno)


@@ -1177,19 +1171,13 @@ static void signal_pre_select(fd_set *rfds, int *max_fileno)
 }
 
 /*
 }
 
 /*

- * 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 fds and stdin. Return when peer has closed both
- * stdout and stderr or when any key is pressed.
+ * This is the core select loop. It checks the following fds:

  *
  *

- * EXTERNAL_MODE: Check only signal pipe. Used when an external command
- * is running. During that time curses is disabled.  Returns when
- * cmd_pid == 0.
+ * - signal pipe
+ * - stdin
+ * - stdout/stderr of display or internal commands

  */
  */

-static int do_select(int mode)
+__noreturn static void do_select(void)

 {
        fd_set rfds;
        int ret, max_fileno;
 {
        fd_set rfds;
        int ret, max_fileno;


@@ -1198,26 +1186,19 @@ static int do_select(int mode)
 repeat:
        tv.tv_sec = conf.timeout_arg  / 1000;
        tv.tv_usec = (conf.timeout_arg % 1000) * 1000;
 repeat:
        tv.tv_sec = conf.timeout_arg  / 1000;
        tv.tv_usec = (conf.timeout_arg % 1000) * 1000;

-//     ret = refresh_status();

        FD_ZERO(&rfds);
        max_fileno = 0;
        status_pre_select(&rfds, &max_fileno, &tv);
        signal_pre_select(&rfds, &max_fileno);
        FD_ZERO(&rfds);
        max_fileno = 0;
        status_pre_select(&rfds, &max_fileno, &tv);
        signal_pre_select(&rfds, &max_fileno);

-       command_pre_select(mode, &rfds, &max_fileno);
-       input_pre_select(mode, &rfds, &max_fileno);
+       command_pre_select(&rfds, &max_fileno);
+       input_pre_select(&rfds, &max_fileno);

        ret = para_select(max_fileno + 1, &rfds, NULL, &tv);
        if (ret <= 0)
        ret = para_select(max_fileno + 1, &rfds, NULL, &tv);
        if (ret <= 0)

-               goto check_return; /* skip fd checks */
+               goto repeat; /* skip fd checks */

        signal_post_select(&rfds);
        signal_post_select(&rfds);

-       /* read command pipe if ready */
-       ret = command_post_select(mode, &rfds);
-       if (ret < 0)
-               return 0;
+       command_post_select(&rfds);

        status_post_select(&rfds);
        status_post_select(&rfds);

-check_return:
-       ret = input_post_select(mode);
-       if (ret != 0)
-               return ret;
+       input_post_select();

        goto repeat;
 }
 
        goto repeat;
 }
 


@@ -1491,8 +1472,6 @@ __noreturn static void print_help_and_die(void)
 
 int main(int argc, char *argv[])
 {
 
 int main(int argc, char *argv[])
 {

-       int ret;
-

        gui_cmdline_parser(argc, argv, &conf); /* exits on errors */
        loglevel = get_loglevel_by_name(conf.loglevel_arg);
        version_handle_flag("gui", conf.version_given);
        gui_cmdline_parser(argc, argv, &conf); /* exits on errors */
        loglevel = get_loglevel_by_name(conf.loglevel_arg);
        version_handle_flag("gui", conf.version_given);


@@ -1504,12 +1483,5 @@ int main(int argc, char *argv[])
        setlocale(LC_CTYPE, "");
        initscr(); /* needed only once, always successful */
        init_curses();
        setlocale(LC_CTYPE, "");
        initscr(); /* needed only once, always successful */
        init_curses();

-       for (;;) {
-               print_status_bar();
-               ret = do_select(GETCH_MODE);
-               if (!ret)
-                       continue;
-               print_in_bar(COLOR_MSG, " ");
-               handle_command(ret);
-       }
+       do_select();

 }
 }