lkml.org 
[lkml]   [2021]   [Nov]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 2/2] tracing: Have existing event_command implementations use helpers
From
Date
Hi Steve,

On Tue, 2021-11-30 at 16:31 -0500, Steven Rostedt wrote:
> On Tue, 23 Nov 2021 14:53:54 -0600
> Tom Zanussi <zanussi@kernel.org> wrote:
>
>
> > index a873f4e8be04..1d1716d5bee2 100644
> > --- a/kernel/trace/trace_events_trigger.c
> > +++ b/kernel/trace/trace_events_trigger.c
> > @@ -931,89 +931,47 @@ event_trigger_callback(struct event_command
> > *cmd_ops,
> > struct event_trigger_data *trigger_data;
> > struct event_trigger_ops *trigger_ops;
> > char *trigger = NULL;
> > - char *number;
> > + bool remove;
> > int ret;
> >
> > - /* separate the trigger from the filter (t:n [if filter]) */
> > - if (param && isdigit(param[0])) {
> > - trigger = strsep(&param, " \t");
> > - if (param) {
> > - param = skip_spaces(param);
> > - if (!*param)
> > - param = NULL;
> > - }
> > - }
> > + remove = event_trigger_check_remove(glob);
> >
> > - trigger_ops = cmd_ops->get_trigger_ops(cmd, trigger);
> > + ret = event_trigger_separate_filter(&trigger, &param, false);
> > + if (ret)
> > + return ret;
> >
> > ret = -ENOMEM;
> > - trigger_data = kzalloc(sizeof(*trigger_data), GFP_KERNEL);
> > + trigger_data = event_trigger_alloc(cmd_ops, cmd, trigger,
> > file);
> > if (!trigger_data)
> > goto out;
> >
> > - trigger_data->count = -1;
> > - trigger_data->ops = trigger_ops;
> > - trigger_data->cmd_ops = cmd_ops;
> > - trigger_data->private_data = file;
> > - INIT_LIST_HEAD(&trigger_data->list);
> > - INIT_LIST_HEAD(&trigger_data->named_list);
> > -
> > - if (glob[0] == '!') {
> > + if (remove) {
> > cmd_ops->unreg(glob+1, trigger_ops, trigger_data,
> > file);
> > - kfree(trigger_data);
>
> How is trigger_data freed on error here?

You're right, that kfree() shouldn't have been removed, will add it
back.

Thanks for picking that up,

Tom

>
> -- Steve
>
> > ret = 0;
> > goto out;
> > }
> >
> > - if (trigger) {
> > - number = strsep(&trigger, ":");
> > -
> > - ret = -EINVAL;
> > - if (!strlen(number))
> > - goto out_free;
> > -
> > - /*
> > - * We use the callback data field (which is a pointer)
> > - * as our counter.
> > - */
> > - ret = kstrtoul(number, 0, &trigger_data->count);
> > - if (ret)
> > - goto out_free;
> > - }
> > -
> > - if (!param) /* if param is non-empty, it's supposed to be a
> > filter */
> > - goto out_reg;
> > -
> > - if (!cmd_ops->set_filter)
> > - goto out_reg;
> > + ret = event_trigger_parse_num(trigger, trigger_data);
> > + if (ret)
> > + goto out_free;
> >
> > - ret = cmd_ops->set_filter(param, trigger_data, file);
> > + ret = event_trigger_set_filter(cmd_ops, file, param,
> > trigger_data);
> > if (ret < 0)
> > goto out_free;
> >
> > - out_reg:
> > /* Up the trigger_data count to make sure reg doesn't free it
> > on failure */
> > event_trigger_init(trigger_ops, trigger_data);
> > - ret = cmd_ops->reg(glob, trigger_ops, trigger_data, file);
> > - /*
> > - * The above returns on success the # of functions enabled,
> > - * but if it didn't find any functions it returns zero.
> > - * Consider no functions a failure too.
> > - */
> > - if (!ret) {
> > - cmd_ops->unreg(glob, trigger_ops, trigger_data, file);
> > - ret = -ENOENT;
> > - } else if (ret > 0)
> > - ret = 0;
> > +
> > + ret = event_trigger_register(cmd_ops, file, glob, cmd, trigger,
> > trigger_data, NULL);
> > + if (ret)
> > + goto out_free;
> >
> > /* Down the counter of trigger_data or free it if not used
> > anymore */
> > event_trigger_free(trigger_ops, trigger_data);
> > out:
> > return ret;
> > -
> > out_free:
> > - if (cmd_ops->set_filter)
> > - cmd_ops->set_filter(NULL, trigger_data, NULL);
> > + event_trigger_reset_filter(cmd_ops, trigger_data);
> > kfree(trigger_data);
> > goto out;
> > }
> >

\
 
 \ /
  Last update: 2021-11-30 22:44    [W:0.056 / U:0.036 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site