This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] [RFC] Don't propagate our current terminal state to the inferior
- From: Patrick Palka <patrick at parcs dot ath dot cx>
- To: gdb-patches at sourceware dot org
- Cc: Patrick Palka <patrick at parcs dot ath dot cx>
- Date: Wed, 7 Jan 2015 08:09:21 -0500
- Subject: Re: [PATCH] [RFC] Don't propagate our current terminal state to the inferior
- Authentication-results: sourceware.org; auth=none
- References: <1416688748-20448-1-git-send-email-patrick at parcs dot ath dot cx>
On Sat, Nov 22, 2014 at 3:39 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> Currently when we start an inferior we have the inferior inherit our
> terminal state. Under TUI, our terminal is highly modified by ncurses
> and readline. So when starting an inferior under TUI, the inferior will
> have a highly modified terminal state which will interfere with standard
> I/O. For example,
>
> $ gdb gdb
> (gdb) break main
> (gdb) run
> (gdb) print puts ("a\nb")
> a
> b
> $1 = 4
> (gdb) [enter TUI mode]
> (gdb) run
> (gdb) [exit TUI mode]
> (gdb) print puts ("a\nb")
> a
> b
> $2 = 4
> (gdb) print puts ("a\r\nb\r")
> a
> b
> $3 = 6
>
> As you can see, when we start the inferior under the regular interface,
> puts() prints the text properly. But when we start the inferior under
> TUI, puts() does not print the text properly. This is because when we
> start the inferior under TUI it inherits our current terminal state
> which has been modified by ncurses to, among other things, require an
> explicit \r\n to print a new line. As a result the inferior performs
> standard I/O in an unexpected way.
>
> Because of this discrepancy, it doesn't seem like a good idea to have
> the inferior inherit our _current_ terminal state for it may have been
> modified by readline and/or ncurses. Instead, we should have the
> inferior inherit a pristine snapshot of our terminal state taken before
> readline or ncurses have had a chance to alter it. This enables the
> inferior to run in a more accurate way, more closely mimicking its
> behavior had the program run standalone. And it fixes the above
> mentioned issue.
>
> I wonder, does this change make sense? What do others think?
>
> Tested on x86_64-unknown-linux-gnu.
>
> * terminal.h (set_initial_inferior_ttystate): Declare.
> * inflow.c (initial_inferior_ttystate): New static variable.
> (set_initial_inferior_ttystate): New setter.
> (child_terminal_init_with_pgrp): Copy initial_inferior_ttystate
> instead of our current terminal state.
> * top.c (gdb_init): Call set_initial_inferior_ttystate.
> ---
> gdb/inflow.c | 13 ++++++++++++-
> gdb/terminal.h | 2 ++
> gdb/top.c | 4 ++++
> 3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/gdb/inflow.c b/gdb/inflow.c
> index 8902174..7b432ad 100644
> --- a/gdb/inflow.c
> +++ b/gdb/inflow.c
> @@ -79,6 +79,10 @@ struct terminal_info
> unimportant. */
> static struct terminal_info our_terminal_info;
>
> +/* The initial tty state given to each new inferior. It is a snapshot of our
> + own tty state taken during initialization of GDB. */
> +static serial_ttystate initial_inferior_ttystate;
> +
> static struct terminal_info *get_inflow_inferior_data (struct inferior *);
>
> #ifdef PROCESS_GROUP_TYPE
> @@ -156,6 +160,13 @@ show_interactive_mode (struct ui_file *file, int from_tty,
> fprintf_filtered (file, "Debugger's interactive mode is %s.\n", value);
> }
>
> +/* Set the initial tty state that is to be inherited by new inferiors. */
> +void
> +set_initial_inferior_ttystate (void)
> +{
> + initial_inferior_ttystate = serial_get_tty_state (stdin_serial);
> +}
> +
> /* Does GDB have a terminal (on stdin)? */
> int
> gdb_has_a_terminal (void)
> @@ -227,7 +238,7 @@ child_terminal_init_with_pgrp (int pgrp)
> {
> xfree (tinfo->ttystate);
> tinfo->ttystate = serial_copy_tty_state (stdin_serial,
> - our_terminal_info.ttystate);
> + initial_inferior_ttystate);
>
> /* Make sure that next time we call terminal_inferior (which will be
> before the program runs, as it needs to be), we install the new
> diff --git a/gdb/terminal.h b/gdb/terminal.h
> index 433aa7d..186bce2 100644
> --- a/gdb/terminal.h
> +++ b/gdb/terminal.h
> @@ -103,6 +103,8 @@ extern int gdb_has_a_terminal (void);
>
> extern void gdb_save_tty_state (void);
>
> +extern void set_initial_inferior_ttystate (void);
> +
> /* Set the process group of the caller to its own pid, or do nothing
> if we lack job control. */
> extern int gdb_setpgid (void);
> diff --git a/gdb/top.c b/gdb/top.c
> index 83d858a..c4b5c2c 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -1886,6 +1886,10 @@ gdb_init (char *argv0)
>
> initialize_stdin_serial ();
>
> + /* Take a snapshot of our tty state before readline/ncurses have had a chance
> + to alter it. */
> + set_initial_inferior_ttystate ();
> +
> async_init_signals ();
>
> /* We need a default language for parsing expressions, so simple
> --
> 2.2.0.rc1.23.gf570943
>
Ping.