Re: [PATCH] [RFC] Don't propagate our current terminal state to the inferior

[Patrick Palka <patrick@parcs.ath.cx>]

On Wed, 7 Jan 2015, Pedro Alves wrote:

> On 11/22/2014 08:39 PM, Patrick Palka 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?
>
> Thanks.  This makes sense to me.
>
>>
>> +/* 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);
>> +}
>
> "initial inferior" reminded me of the initial inferior that
> GDB always creates.  E.g., in main.c:
>
>  /* Now that gdb_init has created the initial inferior, we're in
>     position to set args for that inferior.  */
>
>
> Could you rename the function and the variable?  Like:
>
> set_initial_gdb_ttystate (void)
> {
>  initial_gdb_ttystate = serial_get_tty_state (stdin_serial);
> }
>
> ...
>
> /* Snapshot of our own tty state taken during initialization
>   of GDB.  This is used as tty state given to each new inferior.  */

I noticed that I forgot to accordingly update the comment for
initial_gdb_ttystate as you suggested.  I have committed this trivial
patch under the "obvious" rule:

-- >8 --

Subject: [PATCH] Trivially tweak the comment documenting
  initial_gdb_ttystate

gdb/ChangeLog:

 	* inflow.c (initial_gdb_ttystate): Tweak comment.
---
  gdb/ChangeLog | 4 ++++
  gdb/inflow.c  | 4 ++--
  2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e9b0377..b188988 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,7 @@
+2015-01-07  Patrick Palka  <patrick@parcs.ath.cx>
+
+	* inflow.c (initial_gdb_ttystate): Tweak comment.
+
  2015-01-07  Joel Brobecker  <brobecker@adacore.com>

  	* inflow.c (set_initial_gdb_ttystate): Add empty line after
diff --git a/gdb/inflow.c b/gdb/inflow.c
index 4c81a68..1456fd8 100644
--- a/gdb/inflow.c
+++ b/gdb/inflow.c
@@ -79,8 +79,8 @@ 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.  */
+/* Snapshot of our own tty state taken during initialization of GDB.
+   This is used as the initial tty state given to each new inferior.  */
  static serial_ttystate initial_gdb_ttystate;

  static struct terminal_info *get_inflow_inferior_data (struct inferior *);
-- 
2.2.1.212.gc5b9256