Re: Eliminate tui_command_loop

[Matt Rice <ratmice@gmail.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.