From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: "Jérémie Galarneau" <jeremie.galarneau@efficios.com>
Cc: Wang Nan <wangnan0@huawei.com>, Jiri Olsa <jolsa@redhat.com>,
Sebastian Andrzej Siewior <bigeasy@linutronix.de>,
Li Zefan <lizefan@huawei.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] perf: convert: fix duplicate field names and avoid reserved keywords.
Date: Fri, 23 Jan 2015 20:45:46 +0000 (UTC) [thread overview]
Message-ID: <1074266107.40857.1422045946295.JavaMail.zimbra@efficios.com> (raw)
In-Reply-To: <CA+jJMxu-QPVQTW0rSwUQH0dCy=mZo+0LimnPMVhOoJ9-ybc-9A@mail.gmail.com>
----- Original Message -----
> From: "Jérémie Galarneau" <jeremie.galarneau@efficios.com>
> To: "Wang Nan" <wangnan0@huawei.com>
> Cc: "Jiri Olsa" <jolsa@redhat.com>, "Sebastian Andrzej Siewior" <bigeasy@linutronix.de>, "Li Zefan"
> <lizefan@huawei.com>, linux-kernel@vger.kernel.org, "Mathieu Desnoyers" <mathieu.desnoyers@efficios.com>
> Sent: Wednesday, January 21, 2015 10:14:16 PM
> Subject: Re: [PATCH 1/2] perf: convert: fix duplicate field names and avoid reserved keywords.
>
> On Wed, Jan 21, 2015 at 8:38 PM, Wang Nan <wangnan0@huawei.com> wrote:
> > 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?
> >
>
> Good question. There is discussion around a v1.9 version of the CTF
> spec going on at the moment (which should not affect the Babeltrace
> API).
>
> As far as I know, adding "__attribute__" has been discussed. CC'ing
> Mathieu Desnoyers who may have other extensions in mind.
I've had in mind adding an optional $ prefix to identifiers so they
don't clash with reserved keywords. This would have to go into a
CTF 1.9 though.
Meanwhile, validating that there are no identifier clash in babeltrace
seems like a good idea. Alternatively, prefixing all identifiers with
an underscore eliminates those clashes, and Babeltrace even strip those
underscore before printing, but since underscore is a character that
is allowed within keywords, this can bring interesting clash when
a keyword actually begins with an underscore, so I would like to
replace those by $.
Thoughts ?
Thanks,
Mathieu
>
> Jérémie
>
> >>
> >>> 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
>
--
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
next prev parent reply other threads:[~2015-01-23 20:45 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-01-20 11:07 [PATCH 0/2] perf: convert: two patches for converting syscall tracepoitns Wang Nan
2015-01-20 11:07 ` [PATCH 1/2] perf: convert: fix duplicate field names Wang Nan
2015-01-20 13:06 ` Jiri Olsa
2015-01-21 3:23 ` [PATCH 1/2] perf: convert: fix duplicate field names and avoid reserved keywords Wang Nan
2015-01-21 11:16 ` Wang Nan
2015-01-21 14:19 ` Jiri Olsa
2015-01-21 14:25 ` Steven Rostedt
2015-01-21 14:32 ` Jiri Olsa
2015-01-22 5:35 ` [PATCH RFC 0/2] tools lib traceevent: introduces priv field to struct format_field Wang Nan
2015-01-22 5:36 ` [PATCH RFC 1/2] tools lib traceevent: add priv field to truct format_field Wang Nan
2015-01-22 5:36 ` [PATCH RFC 2/2] perf: convert: fix duplicate field names and avoid reserved keywords Wang Nan
2015-01-22 13:27 ` Jiri Olsa
2015-01-23 1:57 ` Wang Nan
2015-01-23 2:53 ` Wang Nan
2015-01-23 8:44 ` Jiri Olsa
2015-01-23 8:45 ` Jiri Olsa
2015-01-23 22:40 ` Jérémie Galarneau
2015-01-24 16:39 ` Jiri Olsa
2015-01-26 11:11 ` Wang Nan
2015-01-21 14:11 ` [PATCH 1/2] " Jiri Olsa
2015-01-21 15:56 ` Jérémie Galarneau
2015-01-22 1:38 ` Wang Nan
2015-01-22 3:14 ` Jérémie Galarneau
2015-01-22 3:21 ` Wang Nan
2015-01-23 20:45 ` Mathieu Desnoyers [this message]
2015-01-20 11:07 ` [PATCH 2/2] perf: convert: fix signess of value Wang Nan
2015-01-20 13:06 ` [PATCH 0/2] perf: convert: two patches for converting syscall tracepoitns Jiri Olsa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1074266107.40857.1422045946295.JavaMail.zimbra@efficios.com \
--to=mathieu.desnoyers@efficios.com \
--cc=bigeasy@linutronix.de \
--cc=jeremie.galarneau@efficios.com \
--cc=jolsa@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lizefan@huawei.com \
--cc=wangnan0@huawei.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).