[PATCH 1/3] terminal: pop signal handler when terminal is restored
- From
- Phillip Wood via GitGitGadget <gitgitgadget@gmail.com>
- Date
- Feb 16, 2022, 10:54 UTC
- Message-ID
- <9a3c2cea0f904130e944842c3f1d2f31a6b7e95b.1645008873.git.gitgitgadget@gmail.com>
- In-Reply-To
- <pull.1146.git.1645008873.gitgitgadget@gmail.com>
From: Phillip Wood <phillip.wood@dunelm.org.uk>
When disable_bits() changes the terminal attributes it uses sigchain_push_common() to restore the terminal if a signal is received before restore_term() is called. However there is no corresponding call to sigchain_pop_common() when the settings are restored so the signal handler is left on the sigchain stack. This leaves the stack unbalanced so code such as
sigchain_push_common(my_handler); ... read_key_without_echo(...); ... sigchain_pop_common();
pops the handler pushed by disable_bits() rather than the one it intended to. Additionally "git add -p" changes the terminal settings every time it reads a key press so the stack can grow significantly.
In order to fix this save_term() now sets up the signal handler so restore_term() can unconditionally call sigchain_pop_common(). There are no callers of save_term() outside of terminal.c as the only external caller was removed by e3f7e01b50 ("Revert "editor: save and reset terminal after calling EDITOR"", 2021-11-22). Any future callers of save_term() should benefit from having the signal handler set up for them. For example if we reinstate the code to protect against an editor failing to restore the terminal attributes we would want that code to restore the terminal attributes on SIGINT. (As an aside run_command() installs a signal handler that waits for the child before re-raising the signal so we would be guaranteed to restore the settings after the child has exited.)
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> --- compat/terminal.c | 17 +++++++++++++---- compat/terminal.h | 8 ++++++++ 2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/compat/terminal.c b/compat/terminal.c index 5b903e7c7e3..20803badd03 100644 --- a/compat/terminal.c +++ b/compat/terminal.c @@ -11,7 +11,7 @@ static void restore_term_on_signal(int sig) { restore_term(); - sigchain_pop(sig); + /* restore_term calls sigchain_pop_common */ raise(sig); } @@ -31,14 +31,20 @@ void restore_term(void) tcsetattr(term_fd, TCSAFLUSH, &old_term); close(term_fd); term_fd = -1; + sigchain_pop_common(); } int save_term(int full_duplex) { if (term_fd < 0) term_fd = open("/dev/tty", O_RDWR); + if (term_fd < 0) + return -1; + if (tcgetattr(term_fd, &old_term) < 0) + return -1; + sigchain_push_common(restore_term_on_signal); - return (term_fd < 0) ? -1 : tcgetattr(term_fd, &old_term); + return 0; } static int disable_bits(tcflag_t bits) @@ -49,12 +55,12 @@ static int disable_bits(tcflag_t bits) goto error; t = old_term; - sigchain_push_common(restore_term_on_signal); t.c_lflag &= ~bits; if (!tcsetattr(term_fd, TCSAFLUSH, &t)) return 0; + sigchain_pop_common(); error: close(term_fd); term_fd = -1; @@ -100,6 +106,8 @@ void restore_term(void) return; } + sigchain_pop_common(); + if (hconin == INVALID_HANDLE_VALUE) return; @@ -134,6 +142,7 @@ int save_term(int full_duplex) GetConsoleMode(hconin, &cmode_in); use_stty = 0; + sigchain_push_common(restore_term_on_signal); return 0; error: CloseHandle(hconin); @@ -177,10 +186,10 @@ static int disable_bits(DWORD bits) if (save_term(0) < 0) return -1; - sigchain_push_common(restore_term_on_signal); if (!SetConsoleMode(hconin, cmode_in & ~bits)) { CloseHandle(hconin); hconin = INVALID_HANDLE_VALUE; + sigchain_pop_common(); return -1; } diff --git a/compat/terminal.h b/compat/terminal.h index e1770c575b2..0fb9fa147c4 100644 --- a/compat/terminal.h +++ b/compat/terminal.h @@ -1,7 +1,15 @@ #ifndef COMPAT_TERMINAL_H #define COMPAT_TERMINAL_H +/* + * Save the terminal attributes so they can be restored later by a + * call to restore_term(). Note that every successful call to + * save_term() must be matched by a call to restore_term() even if the + * attributes have not been changed. Returns 0 on success, -1 on + * failure. + */ int save_term(int full_duplex); +/* Restore the terminal attributes that were saved with save_term() */ void restore_term(void); char *git_terminal_prompt(const char *prompt, int echo);
-- gitgitgadget