This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Add method/format information to =record-started
- From: Yao Qi <qiyaoltc at gmail dot com>
- To: "Metzger\, Markus T" <markus dot t dot metzger at intel dot com>
- Cc: Simon Marchi <simon dot marchi at ericsson dot com>, "gdb-patches\ at sourceware dot org" <gdb-patches at sourceware dot org>
- Date: Mon, 06 Jun 2016 14:03:12 +0100
- Subject: Re: [PATCH] Add method/format information to =record-started
- Authentication-results: sourceware.org; auth=none
- References: <20160603155220 dot 22286-1-simon dot marchi at ericsson dot com> <A78C989F6D9628469189715575E55B23332EAEF5 at IRSMSX104 dot ger dot corp dot intel dot com>
"Metzger, Markus T" <markus.t.metzger@intel.com> writes:
Hi Markus,
Thanks for reviewing the patch....
>> + if (format != NULL)
>> + {
>
> Do we really need braces, here...
>
Yes, two or more lines in code should be wrapped in braces.
https://sourceware.org/gdb/wiki/Internals%20GDB-C-Coding-Standards
>> @@ -234,7 +235,8 @@ record_btrace_open (const char *args, int from_tty)
>> NULL);
>> record_btrace_generating_corefile = 0;
>>
>> - observer_notify_record_changed (current_inferior (), 1);
>> + format = record_btrace_conf.format == BTRACE_FORMAT_PT ? "pt" : "bts";
>
> I'd use a switch here and not assume that format != pt means bts. If we added
> another format (LBR maybe?) we might miss this.
I agree, otherwise, the patch is good to me.
--
Yao (éå)