more: fix poll() use

The more(1) command utilizes signalfd() to monitor signals and reads
commands from the user via stderr (as stdin is typically used for
piping and not for user interaction).

However, the current more_poll() implementation ignores stderr. As a result,
more(1) waits on read(stderr) while completely ignoring signals. This issue
becomes evident when using commands like:

    grep foo /path/file | more

In such cases, it's only possible to exit 'more' by pressing 'q';
CTRL+C does not work.

Changes:

- Refactor more_poll() code:
  - Introduce an enum to access pfd[] items instead of using magical constants.
  - Implement a while() loop to handle EAGAIN or POLLHUP.

- Ignore stdin after POLLHUP (indicating that the pipe's peer closed).
- Ensure stderr is also checked.
- Use return codes akin to classic poll().

Note: I have doubts regarding the usability of stdin in more_poll(),
as the function is primarily used to wait for user input (via stderr)
and to monitor signals. Nevertheless, it is retained for potential use
in detecting when the pipe's peer (or the entire session) has been
terminated (see commit 68e14d3d5f4116ad3aca0e392d008645ea90cf70).

Signed-off-by: Karel Zak <kzak@redhat.com>

diff --git a/text-utils/more.c b/text-utils/more.c
index a49acbc3e2f091d2c5cadd0869f70a25e98f8458..26a08e3370fdd1a4d682156a651161708d96621e 100644 (file)
--- a/text-utils/more.c
+++ b/text-utils/more.c
@@ -198,6 +198,7 @@ struct more_control {
        magic_t magic;                  /* libmagic database entries */
 #endif
        unsigned int
+               ignore_stdin:1,         /* POLLHUP; peer closed pipe */
                bad_stdout:1,           /* true if overwriting does not turn off standout */
                catch_suspend:1,        /* we should catch the SIGTSTP signal */
                clear_line_ends:1,      /* do not scroll, paint each screen from the top */
@@ -1351,55 +1352,92 @@ static void read_line(struct more_control *ctl)
        *p = '\0';
 }
 
+/* returns: 0 timeout or nothing; <0 error, >0 success */
 static int more_poll(struct more_control *ctl, int timeout)
 {
-       struct pollfd pfd[2];
+       enum {
+               POLLFD_SIGNAL = 0,
+               POLLFD_STDIN,
+               POLLFD_STDERR,
+       };
+       struct pollfd pfd[] = {
+               [POLLFD_SIGNAL] = { .fd = ctl->sigfd,    .events = POLLIN | POLLERR | POLLHUP },
+               [POLLFD_STDIN]  = { .fd = STDIN_FILENO,  .events = POLLIN | POLLERR | POLLHUP },
+               [POLLFD_STDERR] = { .fd = STDERR_FILENO, .events = POLLIN | POLLERR | POLLHUP }
+       };
+       int has_data = 0;
 
-       pfd[0].fd = ctl->sigfd;
-       pfd[0].events = POLLIN | POLLERR | POLLHUP;
-       pfd[1].fd = STDIN_FILENO;
-       pfd[1].events = POLLIN;
+       while (!has_data) {
+               int rc;
 
-       if (poll(pfd, 2, timeout) < 0) {
-               if (errno == EAGAIN)
-                       return 1;
-               more_error(ctl, _("poll failed"));
-               return 1;
-       }
-       if (pfd[0].revents != 0) {
-               struct signalfd_siginfo info;
-               ssize_t sz;
-
-               sz = read(pfd[0].fd, &info, sizeof(info));
-               assert(sz == sizeof(info));
-               switch (info.ssi_signo) {
-               case SIGINT:
-                       more_exit(ctl);
-                       break;
-               case SIGQUIT:
-                       sigquit_handler(ctl);
-                       break;
-               case SIGTSTP:
-                       sigtstp_handler(ctl);
-                       break;
-               case SIGCONT:
-                       sigcont_handler(ctl);
-                       break;
-               case SIGWINCH:
-                       sigwinch_handler(ctl);
-                       break;
-               default:
-                       abort();
+               if (ctl->ignore_stdin)
+                       pfd[POLLFD_STDIN].fd = -1;      /* probably closed, ignore */
+
+               rc = poll(pfd, ARRAY_SIZE(pfd), timeout);
+
+               /* error */
+               if (rc < 0) {
+                       if (errno == EAGAIN)
+                               continue;
+
+                       more_error(ctl, _("poll failed"));
+                       return rc;
                }
-       }
 
-       /* Check for POLLERR and POLLHUP in stdin revents */
-       if ((pfd[1].revents & POLLERR) && (pfd[1].revents & POLLHUP))
-               more_exit(ctl);
+               /* timeout */
+               if (rc == 0)
+                       return 0;
 
-       if (pfd[1].revents == 0)
-               return 1;
-       return 0;
+               /* event on signal FD */
+               if (pfd[POLLFD_SIGNAL].revents) {
+                       struct signalfd_siginfo info;
+                       ssize_t sz;
+
+                       sz = read(pfd[POLLFD_SIGNAL].fd, &info, sizeof(info));
+                       assert(sz == sizeof(info));
+                       switch (info.ssi_signo) {
+                       case SIGINT:
+                               more_exit(ctl);
+                               break;
+                       case SIGQUIT:
+                               sigquit_handler(ctl);
+                               break;
+                       case SIGTSTP:
+                               sigtstp_handler(ctl);
+                               break;
+                       case SIGCONT:
+                               sigcont_handler(ctl);
+                               break;
+                       case SIGWINCH:
+                               sigwinch_handler(ctl);
+                               break;
+                       default:
+                               abort();
+                       }
+               }
+
+               /* event on stdin */
+               if (pfd[POLLFD_STDIN].revents) {
+                       /* Check for POLLERR and POLLHUP in stdin revents */
+                       if ((pfd[POLLFD_STDIN].revents & POLLERR) &&
+                           (pfd[POLLFD_STDIN].revents & POLLHUP))
+                               more_exit(ctl);
+
+                       /* poll() return POLLHUP event after pipe close() and POLLNVAL
+                        * means that fd is already closed. */
+                       if ((pfd[POLLFD_STDIN].revents & POLLHUP) ||
+                           (pfd[POLLFD_STDIN].revents & POLLNVAL))
+                               ctl->ignore_stdin = 1;
+                       else
+                               has_data++;
+               }
+
+               /* event on stderr (we reads user commands from stderr!) */
+               if (pfd[POLLFD_STDERR].revents)
+                       has_data++;
+       }
+
+       return has_data;
 }
 
 /* Search for nth occurrence of regular expression contained in buf in
@@ -1467,7 +1505,7 @@ static void search(struct more_control *ctl, char buf[], int n)
                        }
                        break;
                }
-               more_poll(ctl, 1);
+               more_poll(ctl, 0);
        }
        /* Move ctrl+c signal handling back to more_key_command(). */
        signal(SIGINT, SIG_DFL);
@@ -1631,7 +1669,7 @@ static int more_key_command(struct more_control *ctl, char *filename)
                ctl->report_errors = 0;
        ctl->search_called = 0;
        for (;;) {
-               if (more_poll(ctl, -1) != 0)
+               if (more_poll(ctl, -1) <= 0)
                        continue;
                cmd = read_command(ctl);
                if (cmd.key == more_kc_unknown_command)