From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 13055 invoked by alias); 7 Jan 2015 13:09:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 13044 invoked by uid 89); 7 Jan 2015 13:09:46 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.5 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.2 X-HELO: mail-pa0-f47.google.com Received: from mail-pa0-f47.google.com (HELO mail-pa0-f47.google.com) (209.85.220.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 07 Jan 2015 13:09:44 +0000 Received: by mail-pa0-f47.google.com with SMTP id kq14so4791410pab.6 for ; Wed, 07 Jan 2015 05:09:42 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc:content-type; bh=nRy9BZCKGa2UNg+26THy60Ankfky7ElWabE8LEH7sSQ=; b=btG86DupkPabUwl/hHpkvDhuHbrjDuMXR2+0fTIErWyvo4Khsj9Gyy/79xh3UoYx+Q j8e5SOo3WFklkqSeJU1awBj1mwUAeTUfcGFtDCLfqoL77CJZdP1Q9nCGSy3u2b+yWK8A a2POgM9VaU7/T3rnLJXq+Uxk8nA+sewExIdRI56xzwytaOi06nDE1HRyH+ST0GAKTXvp 8OVQSW3jFnfhE8FoZPqcgZgjsMnF+2LxygSSSQoDANomtdMHkSFQ9pQr1/azpfHlm+7U x6LQg2KkcY163Im004bU/SQVRI1vDpRfX0j9CI1gcWABzbk/FKPwtkOelygfiZjt1+xC Yr4A== X-Gm-Message-State: ALoCoQmHKmsgnGm4Ewq1GD2jpoJ1cI583uKmTGOO8gV3dpoMRL+Jk3vcw5B2tVLirb3JYEPz/pD6 X-Received: by 10.68.225.193 with SMTP id rm1mr5140708pbc.77.1420636182100; Wed, 07 Jan 2015 05:09:42 -0800 (PST) MIME-Version: 1.0 Received: by 10.70.22.145 with HTTP; Wed, 7 Jan 2015 05:09:21 -0800 (PST) In-Reply-To: <1416688748-20448-1-git-send-email-patrick@parcs.ath.cx> References: <1416688748-20448-1-git-send-email-patrick@parcs.ath.cx> From: Patrick Palka Date: Wed, 07 Jan 2015 13:09:00 -0000 Message-ID: Subject: Re: [PATCH] [RFC] Don't propagate our current terminal state to the inferior To: gdb-patches@sourceware.org Cc: Patrick Palka Content-Type: text/plain; charset=UTF-8 X-SW-Source: 2015-01/txt/msg00100.txt.bz2 On Sat, Nov 22, 2014 at 3: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? > > 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.