This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [patch] Dummy first call to gdb_has_a_terminal()
> 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 <rlblaster@gmail.com> 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 <pedro@codesourcery.com> 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
>>
>