Re: [PATCH v5 4/4] tracing: Have existing event_command.parse() implementations use helpers
From: Steven Rostedt
Date: Sat Jan 08 2022 - 19:54:36 EST
On Tue, 14 Dec 2021 12:57:32 -0600
Tom Zanussi <zanussi@xxxxxxxxxx> wrote:
> index da103826f27e..e6a48e8c79eb 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -973,7 +973,7 @@ int event_trigger_register(struct event_command *cmd_ops,
> * @file: The trace_event_file associated with the event
> * @glob: The raw string used to register the trigger
> * @cmd: The cmd portion of the string used to register the trigger
> - * @param: The params portion of the string used to register the trigger
> + * @param_and_filter: The param and filter portion of the string used to register the trigger
> *
> * Common implementation for event command parsing and trigger
> * instantiation.
> @@ -986,94 +986,53 @@ int event_trigger_register(struct event_command *cmd_ops,
> static int
> event_trigger_parse(struct event_command *cmd_ops,
> struct trace_event_file *file,
> - char *glob, char *cmd, char *param)
> + char *glob, char *cmd, char *param_and_filter)
> {
> struct event_trigger_data *trigger_data;
> struct event_trigger_ops *trigger_ops;
> - char *trigger = NULL;
> - char *number;
> + char *param, *filter;
> + bool remove;
> int ret;
>
> - /* separate the trigger from the filter (t:n [if filter]) */
> - if (param && isdigit(param[0])) {
> - trigger = strsep(¶m, " \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);
Did you mean to remove the assignment of trigger_ops here?
> + ret = event_trigger_separate_filter(param_and_filter, ¶m, &filter, false);
> + if (ret)
> + return ret;
>
> ret = -ENOMEM;
> - trigger_data = kzalloc(sizeof(*trigger_data), GFP_KERNEL);
> + trigger_data = event_trigger_alloc(cmd_ops, cmd, param, 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);
It's still used here and below.
I get a warning on this.
Thanks,
-- Steve
> kfree(trigger_data);