From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29868 invoked by alias); 28 Jul 2010 07:28:22 -0000 Received: (qmail 29859 invoked by uid 22791); 28 Jul 2010 07:28:21 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FROM,TW_DB X-Spam-Check-By: sourceware.org Received: from mail-bw0-f41.google.com (HELO mail-bw0-f41.google.com) (209.85.214.41) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 28 Jul 2010 07:28:02 +0000 Received: by bwz9 with SMTP id 9so4563320bwz.0 for ; Wed, 28 Jul 2010 00:27:59 -0700 (PDT) MIME-Version: 1.0 Received: by 10.204.25.151 with SMTP id z23mr7620648bkb.46.1280302079492; Wed, 28 Jul 2010 00:27:59 -0700 (PDT) Received: by 10.204.18.144 with HTTP; Wed, 28 Jul 2010 00:27:59 -0700 (PDT) In-Reply-To: References: <201007201552.03274.pedro@codesourcery.com> Date: Wed, 28 Jul 2010 07:28:00 -0000 Message-ID: Subject: Re: [patch] Dummy first call to gdb_has_a_terminal() From: Balazs Kezes To: Pedro Alves Cc: gdb-patches@sourceware.org Content-Type: text/plain; charset=ISO-8859-1 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/msg00469.txt.bz2 > It works for me. Sorry scratch that, it doesn't work the other way (gdb->gdbtui). I'll look into this when I have a little more time and not already late to work. :) -- Balazs On 7/28/10, Balazs Kezes wrote: > Here is the new patch based on your observations. It works for me. > > -- > Balazs > > Changelog: > * inflow.c (initialize_stdin_serial): Added gdb_has_a_terminal to > actually save all the tty settings. > * top.c (gdb_init): Move initialize_stdin_serial before init_main > because deep down it has some dependencies on initialize_stdin_serial. > > > diff -c -p -r1.59 inflow.c > *** inflow.c 14 May 2010 21:25:51 -0000 1.59 > --- inflow.c 28 Jul 2010 07:04:43 -0000 > *************** void > *** 843,848 **** > --- 843,849 ---- > initialize_stdin_serial (void) > { > stdin_serial = serial_fdopen (0); > + (void) gdb_has_a_terminal (); > } > > void > Index: top.c > =================================================================== > RCS file: /cvs/src/src/gdb/top.c,v > retrieving revision 1.181.2.1 > diff -c -p -r1.181.2.1 top.c > *** top.c 27 Jul 2010 19:13:11 -0000 1.181.2.1 > --- top.c 28 Jul 2010 07:04:44 -0000 > *************** gdb_init (char *argv0) > *** 1623,1631 **** > initialize_inferiors (); > initialize_current_architecture (); > init_cli_cmds(); > - init_main (); /* But that omits this file! Do it now */ > - > initialize_stdin_serial (); > > async_init_signals (); > > --- 1623,1630 ---- > initialize_inferiors (); > initialize_current_architecture (); > init_cli_cmds(); > initialize_stdin_serial (); > + init_main (); /* But that omits this file! Do it now */ > > async_init_signals (); > > > > On 7/20/10, Pedro Alves wrote: >> 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 >> >