This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: Eliminate tui_command_loop
- From: Matt Rice <ratmice at gmail dot com>
- To: Pedro Alves <pedro at codesourcery dot com>
- Cc: gdb-patches at sourceware dot org
- Date: Sat, 10 Sep 2011 05:46:22 -0700
- Subject: Re: Eliminate tui_command_loop
- References: <201108042110.45405.pedro@codesourcery.com>
On Thu, Aug 4, 2011 at 1:10 PM, Pedro Alves <pedro@codesourcery.com> wrote:
> On Wednesday 03 August 2011 19:10:41, Pedro Alves wrote:
>> Looking at the differences between tui/tui-interp.c:tui_command_loop
>> and event-top.c:cli_event_loop,
>> thinking of getting rid of tui_command_loop, the only difference is
>> that tui_command_loop inlines event-loop.c:start_event_loop, and
>> adds this bit:
>>
>> + ? ? ?/* Update gdb output according to TUI mode. ?Since catch_errors
>> + ? ? ? ? preserves the uiout from changing, this must be done at top
>> + ? ? ? ? level of event loop. ?*/
>> + ? ? ?if (tui_active)
>> + ? ? ? ?current_uiout = tui_out;
>> + ? ? ?else
>> + ? ? ? ?current_uiout = tui_old_uiout;
>>
>> I thought, "why don't we just use TRY_CATCH instead of
>> catch_errors then?". ?Turns out we can't as is, because it is
>> TRY_CATCH itself that preserves the uiout from changing...
>
> TRY_CATCH no longer does that. ?I've applied the
> patch below that conceptually does:
Sorry to report that there is an issue here,
Unforunately the story with this current_uiout frobbing doesn't really
end at catch_errors/TRY_CATCH.
because interps.c:interp_set goes:
current_uiout = interp->interpreter_out;
and tui/tui-io.c:tui_initialize_io goes:
tui_old_uiout = current_uiout = cli_out_new (gdb_stdout);
so after tui_initialize_io,
we have a situation where interp->interpreter_out != current_uiout
then, in 'interpreter_exec_cmd'
old_interp = current_interpreter;
....
interp_set (old_interp, 0);
and now current_uiout has now reverted interpreter->interpreter_out.
this then leads to some issue in the future when we try to output something.
saving and restoring current_uiout in interpreter_exec_cmd
seems to work, but I fear its just a piece of bubble gum holding
things together (and further breaks the already broken abstraction),
this comment from captured_mi_execute_command leads me to think that
if the command does actually want to modify the uiout restoring it to
whatever was the current_uiout could be incorrect..
/* Print the result if there were no errors.
Remember that on the way out of executing a command, you have
to directly use the mi_interp's uiout, since the command could
have reset the interpreter, in which case the current uiout
will most likely crash in the mi_out_* routines. */
I suppose the right thing to do is add interp_set_uiout(interp, uiout)
so tui can control whatever its current_interpreter->interpreter_out field is.