Re: [PATCH] PR gdb/13860 - Make MI sync vs async output (closer to) the same.

[Yao Qi <yao at codesourcery dot com>]

The patch looks good.  One question and two suggestions below:

On 05/23/2014 02:14 AM, Pedro Alves wrote:
> Fix this by making run control event printing roughly independent of
> whatever is the current interpreter or uiout.  That is, move these
> prints to interpreter observers, that know whether to print or be
> quiet, and if printing, which uiout to print to.  In the case of the
> console/tui interpreters, only print if the top interpreter.  For MI,
> always print.

Currently, run control prints events directly to uiout and it has to
worry about different interpreters.  Looks your change is like
delegating print events to interpreter and let each interpreter decide
what uiout to use and how to print.  If my understand is correct,
we'd better document somewhere in code that run control should delegate
events printing to interpreter, instead of printing events directly.

> 
> Breakpoint hits / normal stops are already handled similarly -- MI has
> a normal_stop observer that prints the event to both MI and the CLI,
> though that could be cleaned up further in the direction of this
> patch.

Do you mean we can clean up print_stop_event in the same way you did in
this patch?

> diff --git a/gdb/infrun.h b/gdb/infrun.h
> index f0649f3..47509d9 100644
> --- a/gdb/infrun.h
> +++ b/gdb/infrun.h
> @@ -120,6 +120,29 @@ extern int stepping_past_instruction_at (struct address_space *aspace,
>  extern void set_step_info (struct frame_info *frame,
>  			   struct symtab_and_line sal);
>  
> +/* Several print_*_reason helper functions to print why the inferior
> +   has stopped to the passed in UIOUT.  The interpreters call
> +   these.  */
> +
> +/* Signal received, print why the inferior has stopped.  */
> +extern void print_signal_received_reason (struct ui_out *uiout,
> +					  enum gdb_signal siggnal);
> +
> +/* Print why the inferior has stopped.  We are done with a
> +   step/next/si/ni command, print why the inferior has stopped.  */
> +extern void print_end_stepping_range_reason (struct ui_out *uiout);
> +
> +/* The inferior was terminated by a signal, print why it stopped.  */
> +extern void print_signal_exited_reason (struct ui_out *uiout,
> +					enum gdb_signal siggnal);
> +
> +/* The inferior program is finished, print why it stopped.  */
> +extern void print_exited_reason (struct ui_out *uiout, int exitstatus);
> +
> +/* Reverse execution: target ran out of history info, print why the
> +   inferior has stopped.  */
> +extern void print_no_history_reason (struct ui_out *uiout);
> +
>  extern void print_stop_event (struct target_waitstatus *ws);
>  
>  extern int signal_stop_state (int);

I'd like to move these declarations to interps.h or interp-internal.h,
because these functions are only used by different interpreters, and
they can be invisible out side of interpreters.

-- 
Yao (éå)