From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753142AbbDFLwn (ORCPT ); Mon, 6 Apr 2015 07:52:43 -0400 Received: from smtprelay0110.hostedemail.com ([216.40.44.110]:45609 "EHLO smtprelay.hostedemail.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752847AbbDFLwk (ORCPT ); Mon, 6 Apr 2015 07:52:40 -0400 X-Session-Marker: 726F737465647440676F6F646D69732E6F7267 X-Spam-Summary: 2,0,0,,d41d8cd98f00b204,rostedt@goodmis.org,:::::::::::::::::::,RULES_HIT:41:355:379:541:599:800:960:968:973:988:989:1260:1277:1311:1313:1314:1345:1359:1437:1515:1516:1518:1534:1542:1593:1594:1711:1730:1747:1777:1792:2194:2198:2199:2200:2393:2553:2559:2562:2895:2898:3138:3139:3140:3141:3142:3355:3622:3865:3866:3867:3868:3870:3871:3872:3873:3874:4250:4362:5007:6119:6261:7875:7903:7904:8660:9010:10004:10400:10848:10967:11026:11232:11473:11658:11914:12043:12438:12517:12519:12663:12740:13148:13161:13229:13230:13255:13618:14096:14097:21080,0,RBL:none,CacheIP:none,Bayesian:0.5,0.5,0.5,Netcheck:none,DomainCache:0,MSF:not bulk,SPF:fn,MSBL:0,DNSBL:none,Custom_rules:0:0:0 X-HE-Tag: side34_1709766c4214e X-Filterd-Recvd-Size: 3752 Date: Mon, 6 Apr 2015 07:52:37 -0400 From: Steven Rostedt To: Namhyung Kim Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Andrew Morton , Arnaldo Carvalho de Melo , Masami Hiramatsu , Mathieu Desnoyers , Guilherme Cox , Tony Luck , Xie XiuQi Subject: Re: [PATCH 07/18 v3] tracing: Add TRACE_DEFINE_ENUM() macro to map enums to their values Message-ID: <20150406075237.6f1ffe2e@gandalf.local.home> In-Reply-To: <20150406045433.GB15878@danjae.kornet> References: <20150403013802.220157513@goodmis.org> <20150403014123.997385206@goodmis.org> <20150406045433.GB15878@danjae.kornet> X-Mailer: Claws Mail 3.11.1 (GTK+ 2.24.25; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 6 Apr 2015 13:54:33 +0900 Namhyung Kim wrote: > > + if (isalpha(*ptr) || *ptr == '_') { > > + if (strncmp(map->enum_string, ptr, len) == 0 && > > + !isalnum(ptr[len]) && ptr[len] != '_') { > > + ptr = enum_replace(ptr, map, len); > > + /* Hmm, enum string smaller than value */ > > + if (WARN_ON_ONCE(!ptr)) > > + return; > > + /* > > + * No need to decrement here, as enum_replace() > > + * returns the pointer to the character passed > > + * the enum, and two enums can not be placed > > + * back to back without something in between. > > + * We can skip that something in between. > > + */ > > + continue; > > Maybe I'm becoming a bit paranoid, what I worried was like this: > > ENUM1\"ENUM2\" > > In this case, it skips the backslash and makes quotation effective.. The only time a backslash is OK is if it's in a quote, where we do not process enums there anyway. The above isn't valid C outside of quotes, so I'm still not worried. > > > > + } > > + skip_more: > > + do { > > + ptr++; > > + } while (isalnum(*ptr) || *ptr == '_'); > > + /* > > + * If what comes after this variable is a '.' or > > + * '->' then we can continue to ignore that string. > > + */ > > + if (*ptr == '.' || (ptr[0] == '-' && ptr[1] == '>')) { > > + ptr += *ptr == '.' ? 1 : 2; > > + goto skip_more; > > + } > > + /* > > + * Once again, we can skip the delimiter that came > > + * after the string. > > + */ > > + continue; > > + } > > + } > > +} > > + > > +void trace_event_enum_update(struct trace_enum_map **map, int len) > > +{ > > + struct ftrace_event_call *call, *p; > > + const char *last_system = NULL; > > + int last_i; > > + int i; > > + > > + down_write(&trace_event_sem); > > + list_for_each_entry_safe(call, p, &ftrace_events, list) { > > + /* events are usually grouped together with systems */ > > + if (!last_system || call->class->system != last_system) { > > I think simply checking "call->class->system != last_system" would work. I think you are correct, but I'm not sure I want to change it. Mainly because it's more readable that way. The !last_system is basically the "this is first time". Leaving it out may cause people to think it's wrong. But I may change my mind and remove it anyway ;-) If there's other things wrong with this patch, I may update this too. Thanks for reviewing. -- Steve