Re: [PATCH master+7.12] Fix PR20494 - User input stops being echoed in CLI

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2016-08-22 13:48, Pedro Alves wrote:
> This patch fixes a problem that problem triggers if you start an
> inferior, e.g., with the "start" command, in a UI created with the
> new-ui command, and then run a foreground execution command in the
> main UI.  Once the program stops for the latter command, typing in the
> main UI no longer echoes back to the user.
> 
> The problem revolves around this:
> 
> - gdb_has_a_terminal computes its result lazily, on first call.
> 
>   that is what saves gdb's initial main UI terminal state (the UI
>   associated with stdin):
> 
>           our_terminal_info.ttystate = serial_get_tty_state 
> (stdin_serial);
> 
>   This is the state that target_terminal_ours() restores.
> 
> - In this scenario, the gdb_has_a_terminal function happens to be
>   first ever called from within the target_terminal_init call in
>   startup_inferior:
> 
>       (top-gdb) bt
>       #0  gdb_has_a_terminal () at src/gdb/inflow.c:157
>       #1  0x000000000079db22 in child_terminal_init_with_pgrp () at
> src/gdb/inflow.c:217
>        [...]
>       #4  0x000000000065bacb in target_terminal_init () at 
> src/gdb/target.c:456
>       #5  0x00000000004676d2 in startup_inferior () at 
> src/gdb/fork-child.c:531
>        [...]
>       #7  0x000000000046b168 in linux_nat_create_inferior () at
> src/gdb/linux-nat.c:1112
>        [...]
>       #9  0x00000000005f20c9 in start_command (args=0x0, from_tty=1)
> at src/gdb/infcmd.c:657
> 
> If the command to start the inferior is issued on the main UI, then
> readline will have deprepped the terminal when we reach the above, and
> the problem doesn't appear.
> 
> If however the command is issued on a non-main UI, then when we reach
> that gdb_has_a_terminal call, the main UI's terminal state is still
> set to whatever readline has sets it to in rl_prep_terminal, which
> happens to have echo disabled.  Later, when the following synchronous
> execution command finishes, we'll call target_terminal_ours to restore
> gdb's the main UI's terminal settings, and that restores the terminal
> state with echo disabled...
> 
> Conceptually, the fix is to move the gdb_has_a_terminal call earlier,
> to someplace during GDB initialization, before readline/ncurses have
> had a chance to change terminal settings.  Turns out that
> "set_initial_gdb_ttystate" is exactly such a place.
> 
> I say conceptually, because the fix actually inlines the
> gdb_has_a_terminal part that saves the terminal state in
> set_initial_gdb_ttystate and then simplifies gdb_has_a_terminal, since
> there's no point in making gdb_has_a_terminal do lazy computation.

Well, it does fix the problem we observed, so +1!