From 6a7e01542958ff341c07e36e3f0550a64779fca4 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 24 Nov 2023 10:55:19 +0100 Subject: [PATCH 1/4] stat-util: make file name arguments optional in inode_same_at() --- src/basic/stat-util.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/basic/stat-util.c b/src/basic/stat-util.c index c54374b2c9..1783c6eb74 100644 --- a/src/basic/stat-util.c +++ b/src/basic/stat-util.c @@ -187,14 +187,12 @@ int inode_same_at(int fda, const char *filea, int fdb, const char *fileb, int fl struct stat a, b; assert(fda >= 0 || fda == AT_FDCWD); - assert(filea); assert(fdb >= 0 || fdb == AT_FDCWD); - assert(fileb); - if (fstatat(fda, filea, &a, flags) < 0) + if (fstatat(fda, strempty(filea), &a, flags) < 0) return log_debug_errno(errno, "Cannot stat %s: %m", filea); - if (fstatat(fdb, fileb, &b, flags) < 0) + if (fstatat(fdb, strempty(fileb), &b, flags) < 0) return log_debug_errno(errno, "Cannot stat %s: %m", fileb); return stat_inode_same(&a, &b); From 7b3bbb10388a3f549e0a5460ac39b272b18b35e4 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Fri, 24 Nov 2023 10:49:36 +0900 Subject: [PATCH 2/4] ptyfwd: trivial format cleanups --- src/shared/ptyfwd.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index 195e603224..f72cac0a39 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -142,7 +142,7 @@ static bool look_for_escape(PTYForward *f, const char *buffer, size_t n) { if (*p == 0x1D) { usec_t nw = now(CLOCK_MONOTONIC); - if (f->escape_counter == 0 || nw > f->escape_timestamp + ESCAPE_USEC) { + if (f->escape_counter == 0 || nw > f->escape_timestamp + ESCAPE_USEC) { f->escape_timestamp = nw; f->escape_counter = 1; } else { @@ -228,7 +228,7 @@ static int shovel(PTYForward *f) { f->stdin_hangup = true; f->stdin_event_source = sd_event_source_unref(f->stdin_event_source); - } else { + } else { /* Check if ^] has been pressed three times within one second. If we get this we quite * immediately. */ if (look_for_escape(f, f->in_buffer + f->in_buffer_full, k)) @@ -266,12 +266,9 @@ static int shovel(PTYForward *f) { k = read(f->master, f->out_buffer + f->out_buffer_full, LINE_MAX - f->out_buffer_full); if (k < 0) { - /* Note that EIO on the master device - * might be caused by vhangup() or - * temporary closing of everything on - * the other side, we treat it like - * EAGAIN here and try again, unless - * ignore_vhangup is off. */ + /* Note that EIO on the master device might be caused by vhangup() or + * temporary closing of everything on the other side, we treat it like EAGAIN + * here and try again, unless ignore_vhangup is off. */ if (errno == EAGAIN || (errno == EIO && ignore_vhangup(f))) f->master_readable = false; @@ -284,7 +281,7 @@ static int shovel(PTYForward *f) { log_error_errno(errno, "read(): %m"); return pty_forward_done(f, -errno); } - } else { + } else { f->read_from_master = true; f->out_buffer_full += (size_t) k; } From 1b6983b3958b837592d8dc1916a891ba50b0bacd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 24 Nov 2023 10:58:00 +0100 Subject: [PATCH 3/4] ptyfwd: don't reset TTY twice if stdin/stdout point to same TTY And in particular don't save/restore an already reset TTY state. --- src/shared/ptyfwd.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index f72cac0a39..4950932a96 100644 --- a/src/shared/ptyfwd.c +++ b/src/shared/ptyfwd.c @@ -22,6 +22,7 @@ #include "log.h" #include "macro.h" #include "ptyfwd.h" +#include "stat-util.h" #include "terminal-util.h" #include "time-util.h" @@ -477,8 +478,14 @@ int pty_forward_new( (void) ioctl(master, TIOCSWINSZ, &ws); if (!(flags & PTY_FORWARD_READ_ONLY)) { + int same; + assert(f->input_fd >= 0); + same = inode_same_at(f->input_fd, NULL, f->output_fd, NULL, AT_EMPTY_PATH); + if (same < 0) + return same; + if (tcgetattr(f->input_fd, &f->saved_stdin_attr) >= 0) { struct termios raw_stdin_attr; @@ -486,11 +493,14 @@ int pty_forward_new( raw_stdin_attr = f->saved_stdin_attr; cfmakeraw(&raw_stdin_attr); - raw_stdin_attr.c_oflag = f->saved_stdin_attr.c_oflag; - tcsetattr(f->input_fd, TCSANOW, &raw_stdin_attr); + + if (!same) + raw_stdin_attr.c_oflag = f->saved_stdin_attr.c_oflag; + + (void) tcsetattr(f->input_fd, TCSANOW, &raw_stdin_attr); } - if (tcgetattr(f->output_fd, &f->saved_stdout_attr) >= 0) { + if (!same && tcgetattr(f->output_fd, &f->saved_stdout_attr) >= 0) { struct termios raw_stdout_attr; f->saved_stdout = true; @@ -499,7 +509,7 @@ int pty_forward_new( cfmakeraw(&raw_stdout_attr); raw_stdout_attr.c_iflag = f->saved_stdout_attr.c_iflag; raw_stdout_attr.c_lflag = f->saved_stdout_attr.c_lflag; - tcsetattr(f->output_fd, TCSANOW, &raw_stdout_attr); + (void) tcsetattr(f->output_fd, TCSANOW, &raw_stdout_attr); } r = sd_event_add_io(f->event, &f->stdin_event_source, f->input_fd, EPOLLIN|EPOLLET, on_stdin_event, f); From d19ddf91fdaf2661a6fed2908a64f6ae9cd54c50 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 24 Nov 2023 10:50:47 +0100 Subject: [PATCH 4/4] log: when writing a log message to a TTY always end line in CRNL This should make sure our log lines look nice even if the tty we are connected to is in raw mode. Normally, it's the TTY's job to turn an NL we output into a CRNL and interpret it accordingly. However, if the tty is in "raw" mode it won't do that. Specifically, this is controlled by the ONLCR flag on the TTY. A TTY might be in raw mode if our "ptyfwd" logic is used for example, where a 2nd tty is bi-directionally connected to the primary tty, and duplicate processing is not desired. Hence, let's just write out the CR on our own. This will make sure that whenever we output something subsequent output always continues on the beginning of the next line again, regardless the mode the TTY is in. Of course, if the TTY is *not* in raw mode, then the extra CR we now generate is redundant, but it shouldn't hurt either, as it just moves the cursor to the front of the line even though already is just there. We only to that if we actually talk to a TTY though, since we don't want the extra CRs if we are redirected to a pipe or file or so. We are not on Windows after all. Fixes: #30155 --- src/basic/log.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/src/basic/log.c b/src/basic/log.c index 0d78ecfd24..1470611a75 100644 --- a/src/basic/log.c +++ b/src/basic/log.c @@ -53,6 +53,7 @@ static int log_facility = LOG_DAEMON; static bool ratelimit_kmsg = true; static int console_fd = STDERR_FILENO; +static int console_fd_is_tty = -1; /* tri-state: -1 means don't know */ static int syslog_fd = -EBADF; static int kmsg_fd = -EBADF; static int journal_fd = -EBADF; @@ -108,12 +109,14 @@ bool _log_message_dummy = false; /* Always false */ static void log_close_console(void) { /* See comment in log_close_journal() */ (void) safe_close_above_stdio(TAKE_FD(console_fd)); + console_fd_is_tty = -1; } static int log_open_console(void) { if (!always_reopen_console) { console_fd = STDERR_FILENO; + console_fd_is_tty = -1; return 0; } @@ -125,6 +128,7 @@ static int log_open_console(void) { return fd; console_fd = fd_move_above_stdio(fd); + console_fd_is_tty = true; } return 0; @@ -381,6 +385,7 @@ void log_forget_fds(void) { /* Do not call from library code. */ console_fd = kmsg_fd = syslog_fd = journal_fd = -EBADF; + console_fd_is_tty = -1; } void log_set_max_level(int level) { @@ -404,6 +409,16 @@ void log_set_facility(int facility) { log_facility = facility; } +static bool check_console_fd_is_tty(void) { + if (console_fd < 0) + return false; + + if (console_fd_is_tty < 0) + console_fd_is_tty = isatty(console_fd) > 0; + + return console_fd_is_tty; +} + static int write_to_console( int level, int error, @@ -462,7 +477,12 @@ static int write_to_console( iovec[n++] = IOVEC_MAKE_STRING(buffer); if (off) iovec[n++] = IOVEC_MAKE_STRING(off); - iovec[n++] = IOVEC_MAKE_STRING("\n"); + + /* When writing to a TTY we output an extra '\r' (i.e. CR) first, to generate CRNL rather than just + * NL. This is a robustness thing in case the TTY is currently in raw mode (specifically: has the + * ONLCR flag off). We want that subsequent output definitely starts at the beginning of the line + * again, after all. If the TTY is not in raw mode the extra CR should not hurt. */ + iovec[n++] = IOVEC_MAKE_STRING(check_console_fd_is_tty() ? "\r\n" : "\n"); if (writev(console_fd, iovec, n) < 0) {