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(&param, " \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:

> > This seems rather complex. Wouldn't the following work?
> > 
> >     param = skip_spaces(param);
> >     trigger = strsep(&param, " \t");
> >     if (param)
> >             param = strstrip(param);
> >   
> 
> Yes, much better ;-)
> 
> Tom

-- Steve

>  
>       attrs = parse_hist_trigger_attrs(trigger);
>       if (IS_ERR(attrs))
> @@ -4927,6 +4940,7 @@ static int event_hist_trigger_func(struct event_command 
> *cmd_ops,
>       }
>  
>       ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
> +
>       /*
>        * The above returns on success the # of triggers registered,
>        * but if it didn't register any it returns zero.  Consider no

Reply via email to