terminal: don't assume stdin is /dev/tty

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] b061c913bb/ReadKey.xs (L1090)

Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Phillip Wood 2022-03-16 18:54:03 +00:00 committed by Junio C Hamano
parent 02af15dec5
commit e4938ce3cc
2 changed files with 22 additions and 9 deletions

View File

@ -20,28 +20,41 @@ static void restore_term_on_signal(int sig)
#define INPUT_PATH "/dev/tty" #define INPUT_PATH "/dev/tty"
#define OUTPUT_PATH "/dev/tty" #define OUTPUT_PATH "/dev/tty"
static volatile sig_atomic_t term_fd_needs_closing;
static int term_fd = -1; static int term_fd = -1;
static struct termios old_term; 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) void restore_term(void)
{ {
if (term_fd < 0) if (term_fd < 0)
return; return;
tcsetattr(term_fd, TCSAFLUSH, &old_term); tcsetattr(term_fd, TCSAFLUSH, &old_term);
close(term_fd); close_term_fd();
term_fd = -1;
sigchain_pop_common(); sigchain_pop_common();
} }
int save_term(enum save_term_flags flags) int save_term(enum save_term_flags flags)
{ {
if (term_fd < 0) 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) if (term_fd < 0)
return -1; 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; return -1;
}
sigchain_push_common(restore_term_on_signal); sigchain_push_common(restore_term_on_signal);
return 0; return 0;
@ -52,7 +65,7 @@ static int disable_bits(enum save_term_flags flags, tcflag_t bits)
struct termios t; struct termios t;
if (save_term(flags) < 0) if (save_term(flags) < 0)
goto error; return -1;
t = old_term; t = old_term;
@ -65,9 +78,7 @@ static int disable_bits(enum save_term_flags flags, tcflag_t bits)
return 0; return 0;
sigchain_pop_common(); sigchain_pop_common();
error: close_term_fd();
close(term_fd);
term_fd = -1;
return -1; return -1;
} }
@ -362,7 +373,7 @@ int read_key_without_echo(struct strbuf *buf)
static int warning_displayed; static int warning_displayed;
int ch; int ch;
if (warning_displayed || enable_non_canonical(0) < 0) { if (warning_displayed || enable_non_canonical(SAVE_TERM_STDIN) < 0) {
if (!warning_displayed) { if (!warning_displayed) {
warning("reading single keystrokes not supported on " warning("reading single keystrokes not supported on "
"this platform; reading line instead"); "this platform; reading line instead");

View File

@ -4,6 +4,8 @@
enum save_term_flags { enum save_term_flags {
/* Save input and output settings */ /* Save input and output settings */
SAVE_TERM_DUPLEX = 1 << 0, SAVE_TERM_DUPLEX = 1 << 0,
/* Save stdin rather than /dev/tty (fails if stdin is not a terminal) */
SAVE_TERM_STDIN = 1 << 1,
}; };
/* /*