* [PATCH] tracing: Cleanup event_enable_trigger_parse() by using __free()
@ 2026-07-01 21:48 Steven Rostedt
2026-07-01 22:41 ` Masami Hiramatsu
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Steven Rostedt @ 2026-07-01 21:48 UTC (permalink / raw)
To: LKML, Linux Trace Kernel
Cc: Masami Hiramatsu, Mathieu Desnoyers, Markus Elfring
From: Steven Rostedt <rostedt@goodmis.org>
The enable_data variable gets freed on most error paths in
event_enable_trigger_parse(). Use free() to free it and just before
returning normally, call no_free_ptr(enable_data) to keep it from being
freed.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
kernel/trace/trace_events_trigger.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 655db2e82513..fe5ca6ac7c8d 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -1739,7 +1739,7 @@ int event_enable_trigger_parse(struct event_command *cmd_ops,
char *glob, char *cmd, char *param_and_filter)
{
struct trace_event_file *event_enable_file;
- struct enable_trigger_data *enable_data;
+ struct enable_trigger_data *enable_data __free(kfree) = NULL;
struct event_trigger_data *trigger_data;
struct trace_array *tr = file->tr;
char *param, *filter;
@@ -1789,17 +1789,13 @@ int event_enable_trigger_parse(struct event_command *cmd_ops,
enable_data->file = event_enable_file;
trigger_data = trigger_data_alloc(cmd_ops, cmd, param, enable_data);
- if (!trigger_data) {
- kfree(enable_data);
+ if (!trigger_data)
return ret;
- }
if (remove) {
event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
kfree(trigger_data);
- kfree(enable_data);
- ret = 0;
- return ret;
+ return 0;
}
/* Up the trigger_data count to make sure nothing frees it on failure */
@@ -1829,6 +1825,7 @@ int event_enable_trigger_parse(struct event_command *cmd_ops,
goto out_disable;
event_trigger_free(trigger_data);
+ no_free_ptr(enable_data);
return ret;
out_disable:
trace_event_enable_disable(event_enable_file, 0, 1);
@@ -1837,7 +1834,6 @@ int event_enable_trigger_parse(struct event_command *cmd_ops,
out_free:
event_trigger_reset_filter(cmd_ops, trigger_data);
event_trigger_free(trigger_data);
- kfree(enable_data);
return ret;
}
--
2.53.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] tracing: Cleanup event_enable_trigger_parse() by using __free()
2026-07-01 21:48 [PATCH] tracing: Cleanup event_enable_trigger_parse() by using __free() Steven Rostedt
@ 2026-07-01 22:41 ` Masami Hiramatsu
2026-07-02 9:58 ` Markus Elfring
2026-07-02 11:58 ` Markus Elfring
2 siblings, 0 replies; 4+ messages in thread
From: Masami Hiramatsu @ 2026-07-01 22:41 UTC (permalink / raw)
To: Steven Rostedt
Cc: LKML, Linux Trace Kernel, Masami Hiramatsu, Mathieu Desnoyers,
Markus Elfring
On Wed, 1 Jul 2026 17:48:29 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
>
> The enable_data variable gets freed on most error paths in
> event_enable_trigger_parse(). Use free() to free it and just before
> returning normally, call no_free_ptr(enable_data) to keep it from being
> freed.
>
This looks good to me.
Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Thank you,
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> kernel/trace/trace_events_trigger.c | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index 655db2e82513..fe5ca6ac7c8d 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -1739,7 +1739,7 @@ int event_enable_trigger_parse(struct event_command *cmd_ops,
> char *glob, char *cmd, char *param_and_filter)
> {
> struct trace_event_file *event_enable_file;
> - struct enable_trigger_data *enable_data;
> + struct enable_trigger_data *enable_data __free(kfree) = NULL;
> struct event_trigger_data *trigger_data;
> struct trace_array *tr = file->tr;
> char *param, *filter;
> @@ -1789,17 +1789,13 @@ int event_enable_trigger_parse(struct event_command *cmd_ops,
> enable_data->file = event_enable_file;
>
> trigger_data = trigger_data_alloc(cmd_ops, cmd, param, enable_data);
> - if (!trigger_data) {
> - kfree(enable_data);
> + if (!trigger_data)
> return ret;
> - }
>
> if (remove) {
> event_trigger_unregister(cmd_ops, file, glob+1, trigger_data);
> kfree(trigger_data);
> - kfree(enable_data);
> - ret = 0;
> - return ret;
> + return 0;
> }
>
> /* Up the trigger_data count to make sure nothing frees it on failure */
> @@ -1829,6 +1825,7 @@ int event_enable_trigger_parse(struct event_command *cmd_ops,
> goto out_disable;
>
> event_trigger_free(trigger_data);
> + no_free_ptr(enable_data);
> return ret;
> out_disable:
> trace_event_enable_disable(event_enable_file, 0, 1);
> @@ -1837,7 +1834,6 @@ int event_enable_trigger_parse(struct event_command *cmd_ops,
> out_free:
> event_trigger_reset_filter(cmd_ops, trigger_data);
> event_trigger_free(trigger_data);
> - kfree(enable_data);
>
> return ret;
> }
> --
> 2.53.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] tracing: Cleanup event_enable_trigger_parse() by using __free()
2026-07-01 21:48 [PATCH] tracing: Cleanup event_enable_trigger_parse() by using __free() Steven Rostedt
2026-07-01 22:41 ` Masami Hiramatsu
@ 2026-07-02 9:58 ` Markus Elfring
2026-07-02 11:58 ` Markus Elfring
2 siblings, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2026-07-02 9:58 UTC (permalink / raw)
To: linux-trace-kernel, Masami Hiramatsu, Mathieu Desnoyers,
Steven Rostedt
Cc: LKML
…
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -1739,7 +1739,7 @@ int event_enable_trigger_parse(struct event_command *cmd_ops,
> char *glob, char *cmd, char *param_and_filter)
> {
> struct trace_event_file *event_enable_file;
> - struct enable_trigger_data *enable_data;
> + struct enable_trigger_data *enable_data __free(kfree) = NULL;
…
How do you think about to reduce the scope for this local variable?
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] tracing: Cleanup event_enable_trigger_parse() by using __free()
2026-07-01 21:48 [PATCH] tracing: Cleanup event_enable_trigger_parse() by using __free() Steven Rostedt
2026-07-01 22:41 ` Masami Hiramatsu
2026-07-02 9:58 ` Markus Elfring
@ 2026-07-02 11:58 ` Markus Elfring
2 siblings, 0 replies; 4+ messages in thread
From: Markus Elfring @ 2026-07-02 11:58 UTC (permalink / raw)
To: linux-trace-kernel, Masami Hiramatsu, Mathieu Desnoyers,
Steven Rostedt
Cc: LKML
> The enable_data variable gets freed on most error paths in
> event_enable_trigger_parse(). Use free() to free it and just before
> returning normally, call no_free_ptr(enable_data) to keep it from being
> freed.
The conversion is incomplete for the application of scope-based resource management so far.
Regards,
Markus
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-02 11:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-01 21:48 [PATCH] tracing: Cleanup event_enable_trigger_parse() by using __free() Steven Rostedt
2026-07-01 22:41 ` Masami Hiramatsu
2026-07-02 9:58 ` Markus Elfring
2026-07-02 11:58 ` Markus Elfring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox