Messages in this thread | | | Date | Fri, 30 Jan 2015 17:00:38 +0100 | From | Jiri Olsa <> | Subject | Re: [PATCH v2 2/3] perf: convert: fix duplicate field names and avoid reserved keywords. |
| |
On Fri, Jan 30, 2015 at 10:25:37AM -0500, Steven Rostedt wrote: > On Mon, Jan 26, 2015 at 06:38:24PM +0800, Wang Nan wrote: > > (If Steven Rostedt accept the previous patch which introduce a priv > > field to 'struct format_field', we can use a relative simple method > > for name conversion. If not , perf must track name conversion by > > itself.) > > Sorry for coming in so late here. > > > > > 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. > > > > This patch appends '_dupl_X' suffix to avoid problem 1, prepend a '_' > > prefix to avoid problem 2. > > Actually, I don't like this approach. That is, to have this private > data structure. Why not just add an "alias" to format_field. In other > words, instead of hiding this interaction behind a void pointer and > needing to create a function pointer to free it, just add another > field to format field and be done with it. I think it would make > the code a hell of a lot simpler and easier to understand. > > struct format_field { > struct format_field *next; > struct event_format *event; > char *type; > char *name; > + char *alias; > int offset; > int size; > unsigned int arraylen; > unsigned int elementsize; > unsigned long flags; > }; > > And put the logic in event parse to free alias if need be. > Heck, you could even add a pevent_*() func to assign the alias > using strdup, or what not.
ok, sounds good
thanks, jirka
| |