Re: [MI tracepoints 4/9] -trace-start/end/status

[Pedro Alves <pedro at codesourcery dot com>]

On Tuesday 16 March 2010 12:12:33, Vladimir Prus wrote:
> On Monday 15 March 2010 21:46:05 Pedro Alves wrote:
> 
> > Double space after period everywhere please.
> > 
> > +  ui_out_field_int (uiout, "buffer-size",  (int) ts->buffer_size);
> > +  ui_out_field_int (uiout, "buffer-free",  (int) ts->buffer_free);
> > 
> > When these optional fields aren't reported by the target, this
> > will print -1.  I don't see that mentioned in the manual.  Is
> > this intended?  I would have expected optional fields to just
> > not be present, as is common in MI.  trace_status_command does
> > that too.
> 
> I did not realize they are optional. 

All status fields are optional, except for "running", "not running".
The original packet was just:

 @item qTStatus
 Ask the stub if there is a trace experiment running right now.

 Replies:
 @table @samp
 @item T0
 There is no trace experiment running.
 @item T1
 There is a trace experiment running.
 @end table

We then added the support for passing a `:' list of
extra fields to that packet.  See tracepoint.c:parse_trace_status.

> Attached is the revised patch,

> +      gdb_assert (ts->stop_reason != trace_stop_reason_unknown);

Therefore, this assertion is too strong.  It should be a plain 

   if (ts->stop_reason != trace_stop_reason_unknown)
      {

> +
> +      switch (ts->stop_reason)
> +       {
> +       case tstop_command:
> +         stop_reason = "request";
> +         break;
> +       case trace_buffer_full:
> +         stop_reason = "overflow";
> +         break;
> +       case trace_disconnected:
> +         stop_reason = "disconnection";
> +         break;
> +       case tracepoint_passcount:
> +         stop_reason = "passcount";
> +         stopping_tracepoint = ts->stopping_tracepoint;
> +         break;
> +       }
> +
> +      if (stop_reason)
> +       {
> +         ui_out_field_string (uiout, "stop-reason", stop_reason);
> +         if (stopping_tracepoint != -1)
> +           ui_out_field_int (uiout, "stopping-tracepoint",
> +                             stopping_tracepoint);
> +       }
> +    }
> +

And this one should be conditionalized as well:

> +  ui_out_field_int (uiout, "frames", ts->traceframe_count);

And I think the manual patch should be updated to mention
any of these fields might be missing.

-- 
Pedro Alves