On Wed, 2017-10-18 at 12:00 +0900, Namhyung Kim wrote: > Hi Tom, > > On Wed, Oct 04, 2017 at 02:05:17PM -0500, Tom Zanussi wrote: > > Hi Steve, > > > > On Wed, 2017-10-04 at 14:11 -0400, Steven Rostedt wrote: > > > On Fri, 22 Sep 2017 15:00:05 -0500 > > > Tom Zanussi <[email protected]> wrote: > > > > > > > The existing code only allows for one space before and after the 'if' > > > > specifying the filter for a hist trigger. Add code to make that more > > > > permissive as far as whitespace goes. Specifically, we want to allow > > > > spaces in the trigger itself now that we have additional syntax > > > > (onmatch/onmax) where spaces are more natural e.g. spaces after commas > > > > in param lists. > > > > > > > > Signed-off-by: Tom Zanussi <[email protected]> > > > > --- > > > > kernel/trace/trace_events_hist.c | 24 +++++++++++++++++++----- > > > > 1 file changed, 19 insertions(+), 5 deletions(-) > > > > > > > > diff --git a/kernel/trace/trace_events_hist.c > > > > b/kernel/trace/trace_events_hist.c > > > > index ba42fe2..431f2b2 100644 > > > > --- a/kernel/trace/trace_events_hist.c > > > > +++ b/kernel/trace/trace_events_hist.c > > > > @@ -4857,7 +4857,7 @@ static int event_hist_trigger_func(struct > > > > event_command *cmd_ops, > > > > struct synth_event *se; > > > > const char *se_name; > > > > bool remove = false; > > > > - char *trigger; > > > > + char *trigger, *p; > > > > int ret = 0; > > > > > > > > if (!param) > > > > @@ -4866,10 +4866,23 @@ static int event_hist_trigger_func(struct > > > > event_command *cmd_ops, > > > > if (glob[0] == '!') > > > > remove = true; > > > > > > > > - /* separate the trigger from the filter (k:v [if filter]) */ > > > > - trigger = strsep(¶m, " \t"); > > > > - if (!trigger) > > > > - return -EINVAL; > > > > + /* > > > > + * separate the trigger from the filter (k:v [if filter]) > > > > + * allowing for whitespace in the trigger > > > > + */ > > > > + trigger = param; > > > > + p = strstr(param, " if"); > > > > + if (!p) > > > > + p = strstr(param, "\tif"); > > > > + if (p) { > > > > + if (p == trigger) > > > > + return -EINVAL; > > > > + param = p + 1; > > > > + param = strstrip(param); > > > > + *p = '\0'; > > > > + trigger = strstrip(trigger); > > > > + } else > > > > + param = NULL; > > > > > > I think you forgot to update this: > > > > > > > I was going to but on closer inspection realized the simpler form > > wouldn't accomplish the same thing - the problem this is trying to solve > > is to allow bits of whitespace within the trigger (because we now have > > function-like syntax, which should allow whitespace after commas for > > instance) and separating the trigger from the filter ('if'). So we > > explicitly search for 'if' with preceding whitespace, which strsep won't > > accomplish. > > What if a field name is started with 'if' (like "ifb")? I think you > also need to check whitespace after the 'if'. >
Yeah, it will unnecessarily truncate the trigger, will fix. Thanks, Tom > Thanks, > Namhyung
