[1/3] Rename some trace functions
Commit Message
On 12/02/2016 06:47 PM, Simon Marchi wrote:
> On 2016-12-02 11:47, Pedro Alves wrote:
>>> Writing a patch.
>
> I had started a patch as well, but was interrupted by a meeting, so I
> didn't get very far. Thanks for doing it!
>
>> Turns out I quoted the MI names, which are a bit different
>> from the RSP names. Sigh...
>>
>> Anyway, here's what it ends up looking like. I changed
>> the text of the "request" stop reason, because I thought that
>> might be a tiny bit more user friendly for the case of the
>> user using an MI frontend who clicks some "trace stop" button
>> on the GUI instead of running a "command".
>
> The new message "Trace stopped on user request" looks good to me.
>
>> +/* See tracepoint.h. */
>> +
>> +const char *
>> +get_rsp_name (trace_stop_reason reason)
>> +{
>> + return rsp_trace_stop_reason_names[(int) reason];
>
> Should we prefer static_cast<int>() over (int)?
Seems unnecessarily verbose to me when just doing a simple integer
conversion. E.g.:
static_cast<int> (foo) + static_cast<int> (bar) + static_cast<int> (qux)
vs
(int) foo + (int) bar + (int) qux
I don't think there's a real advantage.
>
> It might be a good idea to check that the resulting integer is smaller
> than the array size. If we ever add new stop reasons, we could forget
> to add array elements, so an error() here would catch it.
That's highly unlikely, given that if you're adding a new reason,
you'll need to be able to parse it from RSP, which requires
get_rsp_name.
How about just adding comments:
Thanks,
Pedro Alves
Comments
On 2016-12-02 14:05, Pedro Alves wrote:
> Seems unnecessarily verbose to me when just doing a simple integer
> conversion. E.g.:
>
> static_cast<int> (foo) + static_cast<int> (bar) + static_cast<int>
> (qux)
>
> vs
>
> (int) foo + (int) bar + (int) qux
>
> I don't think there's a real advantage.
I agree.
>> It might be a good idea to check that the resulting integer is smaller
>> than the array size. If we ever add new stop reasons, we could forget
>> to add array elements, so an error() here would catch it.
>
> That's highly unlikely, given that if you're adding a new reason,
> you'll need to be able to parse it from RSP, which requires
> get_rsp_name.
It seems enough then.
@@ -190,6 +190,8 @@ extern void _initialize_tracepoint (void);
static struct trace_status trace_status;
+/* RSP string representation of trace_stop_reason. */
+
static const char *rsp_trace_stop_reason_names[] = {
"tunknown",
"tnotrun",
@@ -70,7 +70,9 @@ struct trace_state_variable
int builtin;
};
-/* The reason why the tracing was stopped last time. */
+/* The reason why the tracing was stopped last time.
+
+ (If you change this, remember to update get_rsp_name as well.) */
enum class trace_stop_reason
{