public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Patrick Palka <patrick@parcs.ath.cx>
To: gdb-patches@sourceware.org
Cc: Patrick Palka <patrick@parcs.ath.cx>
Subject: [PATCH] [RFC] Don't propagate our current terminal state to the inferior
Date: Sat, 22 Nov 2014 20:39:00 -0000	[thread overview]
Message-ID: <1416688748-20448-1-git-send-email-patrick@parcs.ath.cx> (raw)

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

             reply	other threads:[~2014-11-22 20:39 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-22 20:39 Patrick Palka [this message]
2014-11-23 10:25 ` Joel Brobecker
2015-01-07 13:09 ` Patrick Palka
2015-01-07 13:39 ` Pedro Alves
2015-01-07 14:13   ` Patrick Palka
2015-01-07 14:56     ` Joel Brobecker
2015-01-07 14:57       ` Joel Brobecker
2015-01-07 15:18         ` Patrick Palka
2015-01-07 21:44   ` Patrick Palka

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1416688748-20448-1-git-send-email-patrick@parcs.ath.cx \
    --to=patrick@parcs.ath.cx \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).