From: Tom Zanussi <zanussi@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: mhiramat@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 4/4] tracing: Have existing event_command.parse() implementations use helpers
Date: Sat, 08 Jan 2022 22:23:03 -0600 [thread overview]
Message-ID: <b2c8ad2086362a64853b70fc0aa4c29ce6348544.camel@kernel.org> (raw)
In-Reply-To: <20220108195406.0c4f659d@gandalf.local.home>
Hi Steve,
On Sat, 2022-01-08 at 19:54 -0500, Steven Rostedt wrote:
> On Tue, 14 Dec 2021 12:57:32 -0600
> Tom Zanussi <zanussi@kernel.org> 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?
Hmm, yeah, that shouldn't have been removed, but...
>
> > + 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.
I'm not getting a warning, and remove should have crashed the
testcases, but I'm not seeing that either.
Will have to investigate tomorrow..
Tom
>
> Thanks,
>
> -- Steve
>
> > kfree(trigger_data);
next prev parent reply other threads:[~2022-01-09 4:23 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-14 18:57 [PATCH v5 0/4] tracing: Add and use event_command parsing func helpers Tom Zanussi
2021-12-14 18:57 ` [PATCH v5 1/4] tracing: Change event_command func() to parse() Tom Zanussi
2021-12-14 18:57 ` [PATCH v5 2/4] tracing: Change event_trigger_ops func() to trigger() Tom Zanussi
2021-12-14 18:57 ` [PATCH v5 3/4] tracing: Add helper functions to simplify event_command.parse() callback handling Tom Zanussi
2021-12-14 18:57 ` [PATCH v5 4/4] tracing: Have existing event_command.parse() implementations use helpers Tom Zanussi
2022-01-09 0:54 ` Steven Rostedt
2022-01-09 4:23 ` Tom Zanussi [this message]
2022-01-09 17:14 ` Tom Zanussi
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=b2c8ad2086362a64853b70fc0aa4c29ce6348544.camel@kernel.org \
--to=zanussi@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mhiramat@kernel.org \
--cc=rostedt@goodmis.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.