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

[Wang Nan <wangnan0@huawei.com>]

On 2015/1/21 23:56, Jérémie Galarneau wrote:
> On Wed, Jan 21, 2015 at 9:11 AM, Jiri Olsa <jolsa@redhat.com> 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
> 
> 
>