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

[JÃrÃmie Galarneau]

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

> 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



-- 
JÃrÃmie Galarneau
EfficiOS Inc.
http://www.efficios.com
--
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/