Skip to content

Commit e4938ce

Browse files
phillipwoodgitster
authored andcommitted
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] https://github.com/jonathanstowe/TermReadKey/blob/b061c913bbf7ff9bad9b4eea6caae189eacd6063/ReadKey.xs#L1090 Signed-off-by: Phillip Wood <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 02af15d commit e4938ce

File tree

2 files changed

+22
-9
lines changed

2 files changed

+22
-9
lines changed

compat/terminal.c

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,28 +20,41 @@ static void restore_term_on_signal(int sig)
2020
#define INPUT_PATH "/dev/tty"
2121
#define OUTPUT_PATH "/dev/tty"
2222

23+
static volatile sig_atomic_t term_fd_needs_closing;
2324
static int term_fd = -1;
2425
static struct termios old_term;
2526

27+
static void close_term_fd(void)
28+
{
29+
if (term_fd_needs_closing)
30+
close(term_fd);
31+
term_fd_needs_closing = 0;
32+
term_fd = -1;
33+
}
34+
2635
void restore_term(void)
2736
{
2837
if (term_fd < 0)
2938
return;
3039

3140
tcsetattr(term_fd, TCSAFLUSH, &old_term);
32-
close(term_fd);
33-
term_fd = -1;
41+
close_term_fd();
3442
sigchain_pop_common();
3543
}
3644

3745
int save_term(enum save_term_flags flags)
3846
{
3947
if (term_fd < 0)
40-
term_fd = open("/dev/tty", O_RDWR);
48+
term_fd = ((flags & SAVE_TERM_STDIN)
49+
? 0
50+
: open("/dev/tty", O_RDWR));
4151
if (term_fd < 0)
4252
return -1;
43-
if (tcgetattr(term_fd, &old_term) < 0)
53+
term_fd_needs_closing = !(flags & SAVE_TERM_STDIN);
54+
if (tcgetattr(term_fd, &old_term) < 0) {
55+
close_term_fd();
4456
return -1;
57+
}
4558
sigchain_push_common(restore_term_on_signal);
4659

4760
return 0;
@@ -52,7 +65,7 @@ static int disable_bits(enum save_term_flags flags, tcflag_t bits)
5265
struct termios t;
5366

5467
if (save_term(flags) < 0)
55-
goto error;
68+
return -1;
5669

5770
t = old_term;
5871

@@ -65,9 +78,7 @@ static int disable_bits(enum save_term_flags flags, tcflag_t bits)
6578
return 0;
6679

6780
sigchain_pop_common();
68-
error:
69-
close(term_fd);
70-
term_fd = -1;
81+
close_term_fd();
7182
return -1;
7283
}
7384

@@ -362,7 +373,7 @@ int read_key_without_echo(struct strbuf *buf)
362373
static int warning_displayed;
363374
int ch;
364375

365-
if (warning_displayed || enable_non_canonical(0) < 0) {
376+
if (warning_displayed || enable_non_canonical(SAVE_TERM_STDIN) < 0) {
366377
if (!warning_displayed) {
367378
warning("reading single keystrokes not supported on "
368379
"this platform; reading line instead");

compat/terminal.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
enum save_term_flags {
55
/* Save input and output settings */
66
SAVE_TERM_DUPLEX = 1 << 0,
7+
/* Save stdin rather than /dev/tty (fails if stdin is not a terminal) */
8+
SAVE_TERM_STDIN = 1 << 1,
79
};
810

911
/*

0 commit comments

Comments
 (0)