You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
206 lines
5.6 KiB
206 lines
5.6 KiB
5 months ago
|
From 4e8ee013f8cf63572c1d853c015cd4f0dc698ea4 Mon Sep 17 00:00:00 2001
|
||
|
From: Karel Zak <kzak@redhat.com>
|
||
|
Date: Tue, 20 Feb 2024 12:26:33 +0100
|
||
|
Subject: 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>
|
||
|
Upstream: http://github.com/util-linux/util-linux/commit/fe23722854f651984fad597cbb5b44653f72832a
|
||
|
Addresses: https://issues.redhat.com/browse/RHEL-25559
|
||
|
---
|
||
|
text-utils/more.c | 123 +++++++++++++++++++++++++++++++---------------
|
||
|
1 file changed, 83 insertions(+), 40 deletions(-)
|
||
|
|
||
|
diff --git a/text-utils/more.c b/text-utils/more.c
|
||
|
index e2898e52f..a4fbe0220 100644
|
||
|
--- a/text-utils/more.c
|
||
|
+++ b/text-utils/more.c
|
||
|
@@ -199,6 +199,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 */
|
||
|
@@ -1341,50 +1342,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;
|
||
|
}
|
||
|
+
|
||
|
+ /* timeout */
|
||
|
+ if (rc == 0)
|
||
|
+ 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++;
|
||
|
}
|
||
|
- if (pfd[1].revents == 0)
|
||
|
- return 1;
|
||
|
- return 0;
|
||
|
+
|
||
|
+ return has_data;
|
||
|
}
|
||
|
|
||
|
/* Search for nth occurrence of regular expression contained in buf in
|
||
|
@@ -1452,7 +1495,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);
|
||
|
@@ -1616,7 +1659,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)
|
||
|
--
|
||
|
2.46.0
|
||
|
|