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 <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/compat/terminal.c b/compat/terminal.c
index da2f788137b4ecf9001d5908a07fd2a9e77b8fd2..4893294eb67a304393ca21f606a1643044c52f90 100644 (file)
--- 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 aeb24c94783a129058ed47e4e07a4e59da9abefc..79ed00cf61ae1abfa2f433a174dc38ca5fc81706 100644 (file)
--- 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,
 };
 
 /*