[PATCH 1/3] Rename some trace functions

Simon Marchi simon.marchi@polymtl.ca
Fri Dec 2 15:14:00 GMT 2016


On 2016-12-02 06:28, Pedro Alves wrote:
> On 12/01/2016 04:07 PM, Simon Marchi wrote:
>> On 2016-12-01 09:21, Pedro Alves wrote:
>>>>  static void trace_find_command (char *, int);
>>>>  static void trace_find_pc_command (char *, int);
>>>>  static void trace_find_tracepoint_command (char *, int);
>>> 
>>> Surprised you didn't do the same to "trace_find" -> "tfind" ?
>> 
>> Hmm indeed.  And trace_dump -> tdump as well.
>> 
>> I'll send a v2, since there's new stuff.
>> 
>>> Meanwhile we have C++11, so we could consider making this
>>> an "enum class" instead.  Then we could keep using
>>> tstop_command here.
>> 
>> Indeed.  But for this I think I prefer "trace_stop_command" over
>> "tstop_command" anyway.
> 
> Since the command is not called "trace_stop", which is why you're
> changing the function names, that seems inconsistent to me.
> ( MI's trace-stop doesn't count here, I think.  ;-) )

My thought was that the enum value didn't have to match the command 
name.  I read it as, "the command that is used to stop a trace", rather 
than the literal "trace stop" command.

> Let's go with what you have (I'll look at v2), but I'm thinking
> that to that the best would be to make the enum values match the
> remote  counterparts.  Note the current mismatch:
> 
> 	  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;
> 	    case tracepoint_error:
> 	      stop_reason = "error";
> 	      stopping_tracepoint = ts->stopping_tracepoint;
> 	      break;
> 	    }
> 
> 
> @item stop-reason
> Report the reason why the tracing was stopped last time.  This field
> may be absent iff tracing was never stopped on target yet.  The
> value of @samp{request} means the tracing was stopped as result of
> the @code{-trace-stop} command.  The value of @samp{overflow} means
> the tracing buffer is full.  The value of @samp{disconnection} means
> tracing was automatically stopped when @value{GDBN} has disconnected.
> The value of @samp{passcount} means tracing was stopped when a
> tracepoint was passed a maximal number of times for that tracepoint.
> This field is present if @samp{supported} field is not @samp{0}.
> 
> 
> So something like:
> 
>  enum class trace_stop_reason
>  {
>    unknown,
>    never_run,
>    request,
>    overflow,
>    disconnection,
>    passcount,
>    error,
>  };

It would make sense.



More information about the Gdb-patches mailing list