Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755693AbZJFAS7 (ORCPT ); Mon, 5 Oct 2009 20:18:59 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1755096AbZJFAS7 (ORCPT ); Mon, 5 Oct 2009 20:18:59 -0400 Received: from hrndva-omtalb.mail.rr.com ([71.74.56.124]:51178 "EHLO hrndva-omtalb.mail.rr.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754601AbZJFAS6 (ORCPT ); Mon, 5 Oct 2009 20:18:58 -0400 Subject: Re: [PATCH tracing/kprobes v2 2/5] tracing/kprobes: Avoid field name confliction From: Steven Rostedt Reply-To: rostedt@goodmis.org To: Masami Hiramatsu Cc: Frederic Weisbecker , Ingo Molnar , lkml , systemtap , DLE , Thomas Gleixner , Arnaldo Carvalho de Melo , Mike Galbraith , Paul Mackerras , Peter Zijlstra , Christoph Hellwig , Ananth N Mavinakayanahalli , Jim Keniston , "Frank Ch. Eigler" In-Reply-To: <20091002214850.30906.25919.stgit@dhcp-100-2-132.bos.redhat.com> References: <20091002214834.30906.86502.stgit@dhcp-100-2-132.bos.redhat.com> <20091002214850.30906.25919.stgit@dhcp-100-2-132.bos.redhat.com> Content-Type: text/plain Organization: Kihon Technologies Inc. Date: Mon, 05 Oct 2009 20:16:58 -0400 Message-Id: <1254788218.13160.198.camel@gandalf.stny.rr.com> Mime-Version: 1.0 X-Mailer: Evolution 2.26.3 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2851 Lines: 95 On Fri, 2009-10-02 at 17:48 -0400, Masami Hiramatsu wrote: > Check whether the argument name is conflict with other field names. > > Changes in v2: > - Add common_lock_depth to reserved name list. > > Signed-off-by: Masami Hiramatsu > Cc: Frederic Weisbecker > Cc: Ingo Molnar > Cc: Thomas Gleixner > Cc: Arnaldo Carvalho de Melo > Cc: Steven Rostedt > Cc: Mike Galbraith > Cc: Paul Mackerras > Cc: Peter Zijlstra > Cc: Christoph Hellwig > Cc: Ananth N Mavinakayanahalli > Cc: Jim Keniston > Cc: Frank Ch. Eigler > --- > > kernel/trace/trace_kprobe.c | 65 +++++++++++++++++++++++++++++++++++-------- > 1 files changed, 53 insertions(+), 12 deletions(-) > > diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c > index f63ead0..eb1fa0f 100644 > --- a/kernel/trace/trace_kprobe.c > +++ b/kernel/trace/trace_kprobe.c > @@ -38,6 +38,25 @@ > #define MAX_EVENT_NAME_LEN 64 > #define KPROBE_EVENT_SYSTEM "kprobes" > > +/* Reserved field names */ > +#define FIELD_STRING_IP "ip" > +#define FIELD_STRING_NARGS "nargs" > +#define FIELD_STRING_RETIP "ret_ip" > +#define FIELD_STRING_FUNC "func" > + > +const char *reserved_field_names[] = { > + "common_type", > + "common_flags", > + "common_preempt_count", > + "common_pid", > + "common_tgid", > + "common_lock_depth", > + FIELD_STRING_IP, > + FIELD_STRING_NARGS, > + FIELD_STRING_RETIP, > + FIELD_STRING_FUNC, > +}; > + > /* currently, trace_kprobe only supports X86. */ > > struct fetch_func { > @@ -551,6 +570,20 @@ static int parse_probe_arg(char *arg, struct fetch_func *ff, int is_return) > return ret; > } > > +/* Return 1 if name is reserved or already used by another argument */ > +static int conflict_field_name(const char *name, > + struct probe_arg *args, int narg) > +{ > + int i; > + for (i = 0; i < ARRAY_SIZE(reserved_field_names); i++) > + if (!strcmp(reserved_field_names[i], name)) > + return 1; > + for (i = 0; i < narg; i++) > + if (!strcmp(args[i].name, name)) > + return 1; Just a coding preference, but still, I've seen too many mistakes (made them myself too). if (strcmp(args[i].name, name) == 0) Looks better as a match then if (!strcmp(args[i].name, name)) That stands out to me as a miss match. But this is still just a preference and not something to make me argue the patch. -- Steve > + return 0; > +} > + -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/