Make sure terminal settings are restored before exiting
Commit Message
[ Here is a new version of the patch that uses gdb_disable_readline
instead of rl_deprep_terminal or gdb_rl_callback_handler_remove.
A test is added to batch-preserve-term-settings.exp. The test
depends on the cleanup patch I posted earlier. ]
Tested on x86_64 Stretch.
gdb/ChangeLog:
* top.c (quit_force): Undo any modifications that may have been
made to the terminal.
gdb/testsuite/ChangeLog:
* gdb.base/batch-preserve-term-settings.exp
(test_terminal_settings_preserved_after_cli_exit): New test.
---
.../gdb.base/batch-preserve-term-settings.exp | 94 +++++++++++++++++++++-
gdb/top.c | 9 +++
2 files changed, 102 insertions(+), 1 deletion(-)
Comments
(Small request: I find it helpful when the proposed commit
log is always present in patch resubmissions, making patches
self-contained.)
> diff --git a/gdb/top.c b/gdb/top.c
> index 1e30b1c..6df987a 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -66,6 +66,7 @@
> #include "cli-out.h"
> #include "tracepoint.h"
> #include "inf-loop.h"
> +#include "tui/tui.h"
>
> extern void initialize_all_files (void);
>
> @@ -1494,6 +1495,14 @@ quit_force (char *args, int from_tty)
> int exit_code = 0;
> struct qt_args qt;
>
> + /* Make sure that the terminal is left in the state we acquired it. */
> + target_terminal_ours ();
> +#if defined(TUI)
> + tui_disable ();
> +#endif
> + if (async_command_editing_p)
> + gdb_disable_readline ();
> +
Can you put these new bits in a separate function, and call it here?
That'll make it easier to reuse, or play with moving the call elsewhere,
if needed. As bonus, if we do the latter, git blame on the function's contents
will still blame you. :-)
OK with that change.
Thanks,
Pedro Alves
On Tue, Jul 28, 2015 at 7:53 PM, Pedro Alves <palves@redhat.com> wrote:
> (Small request: I find it helpful when the proposed commit
> log is always present in patch resubmissions, making patches
> self-contained.)
Sure, no problem.
>
>> diff --git a/gdb/top.c b/gdb/top.c
>> index 1e30b1c..6df987a 100644
>> --- a/gdb/top.c
>> +++ b/gdb/top.c
>> @@ -66,6 +66,7 @@
>> #include "cli-out.h"
>> #include "tracepoint.h"
>> #include "inf-loop.h"
>> +#include "tui/tui.h"
>>
>> extern void initialize_all_files (void);
>>
>> @@ -1494,6 +1495,14 @@ quit_force (char *args, int from_tty)
>> int exit_code = 0;
>> struct qt_args qt;
>>
>> + /* Make sure that the terminal is left in the state we acquired it. */
>> + target_terminal_ours ();
>> +#if defined(TUI)
>> + tui_disable ();
>> +#endif
>> + if (async_command_editing_p)
>> + gdb_disable_readline ();
>> +
>
> Can you put these new bits in a separate function, and call it here?
> That'll make it easier to reuse, or play with moving the call elsewhere,
> if needed. As bonus, if we do the latter, git blame on the function's contents
> will still blame you. :-)
>
> OK with that change.
Will do.
@@ -176,4 +176,96 @@ proc test_terminal_settings_preserved {} {
exit_shell
}
-test_terminal_settings_preserved
+# Check that quitting from the CLI via the "quit" command does not leave the
+# terminal in the wrong state. The GDB commands CMDS are executed before
+# quitting.
+
+proc test_terminal_settings_preserved_after_cli_exit { cmds } {
+ global file_arg
+ global GDB INTERNAL_GDBFLAGS GDBFLAGS
+ global gdb_prompt
+ global shell_prompt_re
+
+ if ![spawn_shell] {
+ return
+ }
+
+ set saved_gdbflags $GDBFLAGS
+
+ set stty_supported [run_stty "stty before" stty_before]
+
+ set test "start gdb"
+ send_gdb "$GDB $INTERNAL_GDBFLAGS $GDBFLAGS [host_info gdb_opts] --args \"$file_arg\"\n"
+ gdb_expect {
+ -re "$gdb_prompt $" {
+ pass $test
+ }
+ timeout {
+ fail "$test (timeout)"
+ }
+ eof {
+ fail "$test (eof)"
+ }
+ }
+
+ foreach cmd $cmds {
+ set test "run command $cmd"
+ send_gdb "$cmd\n"
+ gdb_expect {
+ -re "$gdb_prompt $" {
+ pass $test
+ }
+ timeout {
+ fail "$test (timeout)"
+ }
+ eof {
+ fail "$test (eof)"
+ }
+ }
+ }
+
+ set test "quit gdb"
+ send_gdb "quit\n"
+ gdb_expect {
+ -re "(y or n)" {
+ send_gdb "y\n"
+ exp_continue
+ }
+ -re ".*$shell_prompt_re$" {
+ pass $test
+ }
+ timeout {
+ fail "$test (timeout)"
+ }
+ eof {
+ fail "$test (eof)"
+ }
+ }
+
+ set test "terminal settings preserved"
+ if $stty_supported {
+ run_stty "stty after" stty_after
+
+ gdb_assert [string equal $stty_before $stty_after] $test
+ } else {
+ unsupported "$test (no stty)"
+ }
+
+ exit_shell
+}
+
+with_test_prefix "batch run" {
+ test_terminal_settings_preserved
+}
+
+with_test_prefix "cli exit" {
+ test_terminal_settings_preserved_after_cli_exit { }
+}
+
+with_test_prefix "cli exit after start cmd" {
+ test_terminal_settings_preserved_after_cli_exit { "start" }
+}
+
+with_test_prefix "cli exit after run cmd" {
+ test_terminal_settings_preserved_after_cli_exit { "run" }
+}
@@ -66,6 +66,7 @@
#include "cli-out.h"
#include "tracepoint.h"
#include "inf-loop.h"
+#include "tui/tui.h"
extern void initialize_all_files (void);
@@ -1494,6 +1495,14 @@ quit_force (char *args, int from_tty)
int exit_code = 0;
struct qt_args qt;
+ /* Make sure that the terminal is left in the state we acquired it. */
+ target_terminal_ours ();
+#if defined(TUI)
+ tui_disable ();
+#endif
+ if (async_command_editing_p)
+ gdb_disable_readline ();
+
/* An optional expression may be used to cause gdb to terminate with the
value of that expression. */
if (args)