Re: [PATCH 1/2] perf: convert: fix duplicate field names and avoid reserved keywords.

From: Wang Nan
Date: Wed Jan 21 2015 - 20:38:36 EST


On 2015/1/21 23:56, JÃrÃmie Galarneau wrote:
> On Wed, Jan 21, 2015 at 9:11 AM, Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
>> On Wed, Jan 21, 2015 at 11:23:54AM +0800, Wang Nan wrote:
>>> Some parameters of syscall tracepoints named as 'nr', 'event', etc.
>>> When dealing with them, perf convert to ctf meets some problem:
>>>
>>> 1. If a parameter with name 'nr', it will duplicate syscall's
>>> common field 'nr'. One such syscall is io_submit().
>>>
>>> 2. If a parameter with name 'event', it is denied to be inserted
>>> because 'event' is a babeltrace keywork. One such syscall is
>>> epoll_ctl.
>>
>> hum, so this problem 2 is detectable only via bt_ctf_event_class_add_field function?
>>
>> how big is the blaklist?
>>
>
> The blacklist is defined by the CTF specification here [1].
>
> JÃrÃmie
>
> [1] http://git.efficios.com/?p=ctf.git;a=blob;f=common-trace-format-specification.txt;h=abe4fb70fff7f17f6e8242f313fb74bff44cf89a;hb=HEAD#l1477

Is there any possibility that the someone expand the list?

>
>> SNIP
>>
>>> +}
>>> +
>>> static int add_tracepoint_fields_types(struct ctf_writer *cw,
>>> struct format_field *fields,
>>> struct bt_ctf_event_class *event_class)
>>> @@ -577,6 +609,9 @@ static int add_tracepoint_fields_types(struct ctf_writer *cw,
>>> for (field = fields; field; field = field->next) {
>>> struct bt_ctf_field_type *type;
>>> unsigned long flags = field->flags;
>>> + struct bt_ctf_field_type *f = NULL;
>>> + char *name;
>>> + int dup = 1;
>>>
>>> pr2(" field '%s'\n", field->name);
>>>
>>> @@ -595,14 +630,36 @@ static int add_tracepoint_fields_types(struct ctf_writer *cw,
>>> if (flags & FIELD_IS_ARRAY)
>>> type = bt_ctf_field_type_array_create(type, field->arraylen);
>>>
>>> - ret = bt_ctf_event_class_add_field(event_class, type,
>>> - field->name);
>>> + /* Check name duplication */
>>> + name = field->name;
>>
>> could you please put this in separated function like 'get_field_name(..)'
>> so we dont polute this function even more
>>
>> name == get_field_name(...)
>> if (!name)
>> error path
>>
>>
>> thanks,
>> jirka
>
>
>


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/