From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 20309 invoked by alias); 20 Jul 2010 14:52:19 -0000 Received: (qmail 20299 invoked by uid 22791); 20 Jul 2010 14:52:17 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,TW_DB,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 20 Jul 2010 14:52:11 +0000 Received: (qmail 8729 invoked from network); 20 Jul 2010 14:52:09 -0000 Received: from unknown (HELO orlando.localnet) (pedro@127.0.0.2) by mail.codesourcery.com with ESMTPA; 20 Jul 2010 14:52:09 -0000 From: Pedro Alves To: gdb-patches@sourceware.org Subject: Re: [patch] Dummy first call to gdb_has_a_terminal() Date: Tue, 20 Jul 2010 14:52:00 -0000 User-Agent: KMail/1.13.2 (Linux/2.6.31-11-rt; KDE/4.4.2; x86_64; ; ) Cc: Balazs Kezes References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201007201552.03274.pedro@codesourcery.com> X-IsSubscribed: yes 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 X-SW-Source: 2010-07/txt/msg00300.txt.bz2 On Sunday 18 July 2010 01:30:40, Balazs Kezes wrote: > Hi, > > I've noticed a weird behaviour with TUI at home and at work. It appears when I > switch to the other mode (to command line from gdbtui or to tui from gdb). It > does fix itself when I manage to switch back, but sometimes after executing a > command it messes up readline so I can't switch back easily. > > For example: > > gdbtui /bin/echo > ^x^a > run > > The input is messed up now on my computer. > > I think bug gdb/9294 is exactly this. > > I've tracked it down to gdb_has_a_terminal(). Deep in tui_enable() this > function this called. At this time the terminal has a messed up state (for > example echo is disabled) which is fine. But it turns out this is the first > call to this function and therefore it saves the current terminal settings > which will be used to restore the terminal before displaying the prompt after > executing a command. Even though it works for the current mode, it doesn't > for the other. A neutral mode (the state when gdb starts up) seems to work for > both modes. > > The fix is to have a dummy first call somewhere where the terminal is still in > sane state. Thanks for investigating this. This bug annoys me too. > > Cheers, > Balazs > > Index: tui-interp.c > =================================================================== > RCS file: /cvs/src/src/gdb/tui/tui-interp.c,v > retrieving revision 1.27 > diff -c -p -r1.27 tui-interp.c > *** tui-interp.c 17 May 2010 22:21:43 -0000 1.27 > --- tui-interp.c 18 Jul 2010 00:29:25 -0000 > *************** _initialize_tui_interp (void) > *** 224,229 **** > --- 224,232 ---- > tui_command_loop, > }; > > + /* Dummy first call to save sane terminal settings. */ > + (void) gdb_has_a_terminal (); > + Unfortunately, this is not a good place for this call. See the comment in inflow.c: /* Does GDB have a terminal (on stdin)? */ int gdb_has_a_terminal (void) { switch (gdb_has_a_terminal_flag) { case yes: return 1; case no: return 0; case have_not_checked: /* Get all the current tty settings (including whether we have a tty at all!). Can't do this in _initialize_inflow because serial_fdopen() won't work until the serial_ops_list is initialized. */ Two issues here: First, you can't rely on the order the _initialize_XXX functions are called. In fact, on my gdb build, all the serial_add_interface calls have already happened by the time _initialize_inflow is called (meaning serial_ops_list is already fully initialized), so per-chance, moving your call to _initialize_inflow _would_ work for me. But there's no garantee it will keep working, and neither does adding the call in _initialize_tui_interp. Second, your new call happens before stdin_serial has been initialized by initialize_stdin_serial, called from gdb_init: /* Get all the current tty settings (including whether we have a tty at all!). We can't do this in _initialize_inflow because serial_fdopen() won't work until the serial_ops_list is initialized, but we don't want to do it lazily either, so that we can guarantee stdin_serial is opened if there is a terminal. */ void initialize_stdin_serial (void) { stdin_serial = serial_fdopen (0); } That means that gdb_has_a_terminal would always return false with your patch, given the (stdin_serial != NULL) check it has: /* Does GDB have a terminal (on stdin)? */ int gdb_has_a_terminal (void) { switch (gdb_has_a_terminal_flag) { case yes: return 1; case no: return 0; case have_not_checked: ... gdb_has_a_terminal_flag = no; if (stdin_serial != NULL) { our_terminal_info.ttystate = serial_get_tty_state (stdin_serial); if (our_terminal_info.ttystate != NULL) { gdb_has_a_terminal_flag = yes; It would seem better to add the new call in initialize_stdin_serial. In fact the "(including whether we have a tty at all!)" comment makes it sound like that was also the intent. I don't know the history of this function. Does that work? We may need to tweak the orders in gdb_init as well, not sure. -- Pedro Alves