[PATCH v4 2/4] terminal: don't assume stdin is /dev/tty
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Mar 16, 2022, 18:54 UTC
- Message-ID
- <20220316185405.29551-3-phillip.wood123@gmail.com>
- In-Reply-To
- <20220316185405.29551-1-phillip.wood123@gmail.com>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
read_key_without_echo() reads from stdin but uses /dev/tty when it disables echo. This is unfortunate as there no guarantee that stdin is the same device as /dev/tty. The perl version of "add -p" uses stdin when it sets the terminal mode, this commit does the same for the builtin version. There is still a difference between the perl and builtin versions though - the perl version will ignore any errors when setting the terminal mode[1] and will still read single bytes when stdin is not a terminal. The builtin version displays a warning if setting the terminal mode fails and switches to reading a line at a time.
[1] https://github.com/jonathanstowe/TermReadKey/blob/b061c913bbf7ff9bad9b4eea6caae189eacd6063/ReadKey.xs#L1090
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- compat/terminal.c | 29 ++++++++++++++++++++--------- compat/terminal.h | 2 ++ 2 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/compat/terminal.c b/compat/terminal.c index da2f788137..4893294eb6 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -20,28 +20,41 @@ static void restore_term_on_signal(int sig) #define INPUT_PATH "/dev/tty" #define OUTPUT_PATH "/dev/tty" +static volatile sig_atomic_t term_fd_needs_closing; static int term_fd = -1; static struct termios old_term; +static void close_term_fd(void) +{ + if (term_fd_needs_closing) + close(term_fd); + term_fd_needs_closing = 0; + term_fd = -1; +} + void restore_term(void) { if (term_fd < 0) return; tcsetattr(term_fd, TCSAFLUSH, &old_term); - close(term_fd); - term_fd = -1; + close_term_fd(); sigchain_pop_common(); } int save_term(enum save_term_flags flags) { if (term_fd < 0) - term_fd = open("/dev/tty", O_RDWR); + term_fd = ((flags & SAVE_TERM_STDIN) + ? 0 + : open("/dev/tty", O_RDWR)); if (term_fd < 0) return -1; - if (tcgetattr(term_fd, &old_term) < 0) + term_fd_needs_closing = !(flags & SAVE_TERM_STDIN); + if (tcgetattr(term_fd, &old_term) < 0) { + close_term_fd(); return -1; + } sigchain_push_common(restore_term_on_signal); return 0; @@ -52,7 +65,7 @@ static int disable_bits(enum save_term_flags flags, tcflag_t bits) struct termios t; if (save_term(flags) < 0) - goto error; + return -1; t = old_term; @@ -65,9 +78,7 @@ static int disable_bits(enum save_term_flags flags, tcflag_t bits) return 0; sigchain_pop_common(); -error: - close(term_fd); - term_fd = -1; + close_term_fd(); return -1; } @@ -362,7 +373,7 @@ int read_key_without_echo(struct strbuf *buf) static int warning_displayed; int ch; - if (warning_displayed || enable_non_canonical(0) < 0) { + if (warning_displayed || enable_non_canonical(SAVE_TERM_STDIN) < 0) { if (!warning_displayed) { warning("reading single keystrokes not supported on " "this platform; reading line instead"); diff --git a/compat/terminal.h b/compat/terminal.h index aeb24c9478..79ed00cf61 100644 --- a/compat/terminal.h +++ b/compat/terminal.h @@ -4,6 +4,8 @@ enum save_term_flags { /* Save input and output settings */ SAVE_TERM_DUPLEX = 1 << 0, + /* Save stdin rather than /dev/tty (fails if stdin is not a terminal) */ + SAVE_TERM_STDIN = 1 << 1, }; /*
-- 2.35.1