linux-kernel - Re: [PATCH v5 4/4] tracing: Have existing event_command.parse() implementations use helpers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220108195406.0c4f659d@gandalf.local.home>
Date:   Sat, 8 Jan 2022 19:54:06 -0500
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Tom Zanussi <zanussi@...nel.org>
Cc:     mhiramat@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 4/4] tracing: Have existing event_command.parse()
 implementations use helpers

On Tue, 14 Dec 2021 12:57:32 -0600
Tom Zanussi <zanussi@...nel.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(&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);

Did you mean to remove the assignment of trigger_ops here?

> +	ret = event_trigger_separate_filter(param_and_filter, &param, &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);