From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1762694AbbA3PZm (ORCPT ); Fri, 30 Jan 2015 10:25:42 -0500 Received: from cdptpa-outbound-snat.email.rr.com ([107.14.166.228]:41002 "EHLO cdptpa-oedge-vip.email.rr.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1750852AbbA3PZk (ORCPT ); Fri, 30 Jan 2015 10:25:40 -0500 Date: Fri, 30 Jan 2015 10:25:37 -0500 From: Steven Rostedt To: Wang Nan Cc: jolsa@redhat.com, jeremie.galarneau@efficios.com, alexmonthy@voxpopuli.im, bigeasy@linutronix.de, lizefan@huawei.com, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/3] perf: convert: fix duplicate field names and avoid reserved keywords. Message-ID: <20150130152537.GA10543@home.goodmis.org> References: <1422268705-32084-1-git-send-email-wangnan0@huawei.com> <1422268705-32084-3-git-send-email-wangnan0@huawei.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1422268705-32084-3-git-send-email-wangnan0@huawei.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-RR-Connecting-IP: 107.14.168.118:25 X-Cloudmark-Score: 0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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. -- Steve