This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


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


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]