* [PATCH v4] kernel/trace: fixed static warnings
@ 2026-04-06 7:28 abhijithsriram95
2026-04-08 20:24 ` Steven Rostedt
0 siblings, 1 reply; 3+ messages in thread
From: abhijithsriram95 @ 2026-04-06 7:28 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
open list:TRACING, open list:TRACING
Cc: Abhijith Sriram
From: Abhijith Sriram <abhijithsriram95@gmail.com>
The change in the function argument description
was due to the static code checker script reading
the word filter back to back
Changes in v2:
- corrected *m = file->private_data to m = file->private_data
Changes in v3:
- reverted the changes for struct seq_file *m and
added a new empty line instead
Changes in v4:
- added a new empty line before char *buf ...
previously this line was relocated to avoid the
static check warning.
Signed-off-by: Abhijith Sriram <abhijithsriram95@gmail.com>
---
kernel/trace/trace_events_trigger.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 655db2e82513..664283bcd9ea 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -246,7 +246,7 @@ event_triggers_post_call(struct trace_event_file *file,
}
EXPORT_SYMBOL_GPL(event_triggers_post_call);
-#define SHOW_AVAILABLE_TRIGGERS (void *)(1UL)
+#define SHOW_AVAILABLE_TRIGGERS ((void *)(1UL))
static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
{
@@ -352,6 +352,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
ret = seq_open(file, &event_triggers_seq_ops);
if (!ret) {
struct seq_file *m = file->private_data;
+
m->private = file;
}
}
@@ -390,6 +391,7 @@ static ssize_t event_trigger_regex_write(struct file *file,
{
struct trace_event_file *event_file;
ssize_t ret;
+
char *buf __free(kfree) = NULL;
if (!cnt)
@@ -633,6 +635,7 @@ clear_event_triggers(struct trace_array *tr)
list_for_each_entry(file, &tr->events, list) {
struct event_trigger_data *data, *n;
+
list_for_each_entry_safe(data, n, &file->triggers, list) {
trace_event_trigger_enable_disable(file, 0);
list_del_rcu(&data->list);
@@ -785,7 +788,7 @@ static void unregister_trigger(char *glob,
* cmd - the trigger command name
* glob - the trigger command name optionally prefaced with '!'
* param_and_filter - text following cmd and ':'
- * param - text following cmd and ':' and stripped of filter
+ * param - text following cmd and ':' and filter removed
* filter - the optional filter text following (and including) 'if'
*
* To illustrate the use of these components, here are some concrete
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v4] kernel/trace: fixed static warnings
2026-04-06 7:28 [PATCH v4] kernel/trace: fixed static warnings abhijithsriram95
@ 2026-04-08 20:24 ` Steven Rostedt
2026-04-09 12:07 ` Abhijith Sriram
0 siblings, 1 reply; 3+ messages in thread
From: Steven Rostedt @ 2026-04-08 20:24 UTC (permalink / raw)
To: abhijithsriram95
Cc: Masami Hiramatsu, Mathieu Desnoyers, open list:TRACING,
open list:TRACING
Subject should be:
tracing: Fixed static checker warnings
On Mon, 6 Apr 2026 09:28:34 +0200
abhijithsriram95@gmail.com wrote:
> From: Abhijith Sriram <abhijithsriram95@gmail.com>
>
> The change in the function argument description
> was due to the static code checker script reading
> the word filter back to back
>
The below changes should be beneath the '---'
> Changes in v2:
The last change should be first. In fact, I only care about the last change
as the previous versions should have the description of what changed.
> - corrected *m = file->private_data to m = file->private_data
>
> Changes in v3:
> - reverted the changes for struct seq_file *m and
> added a new empty line instead
>
> Changes in v4:
That said, this should really be:
Changes since v3: https://lore.kernel.org/all/20260406060046.223496-2-abhijithsriram95@gmail.com/
> - added a new empty line before char *buf ...
> previously this line was relocated to avoid the
> static check warning.
>
> Signed-off-by: Abhijith Sriram <abhijithsriram95@gmail.com>
> ---
> kernel/trace/trace_events_trigger.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> index 655db2e82513..664283bcd9ea 100644
> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -246,7 +246,7 @@ event_triggers_post_call(struct trace_event_file *file,
> }
> EXPORT_SYMBOL_GPL(event_triggers_post_call);
>
> -#define SHOW_AVAILABLE_TRIGGERS (void *)(1UL)
> +#define SHOW_AVAILABLE_TRIGGERS ((void *)(1UL))
>
> static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
> {
> @@ -352,6 +352,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
> ret = seq_open(file, &event_triggers_seq_ops);
> if (!ret) {
> struct seq_file *m = file->private_data;
> +
This blank line makes the code look worse. Yes, we usually want a blank
line between the variable declarations and the code, but when it comes to
code blocks (not functions) that rule is not as strict.
Get rid of this newline.
> m->private = file;
> }
> }
> @@ -390,6 +391,7 @@ static ssize_t event_trigger_regex_write(struct file *file,
> {
> struct trace_event_file *event_file;
> ssize_t ret;
> +
> char *buf __free(kfree) = NULL;
The char *buf is a declaration. It no new line is expected before it.
>
> if (!cnt)
> @@ -633,6 +635,7 @@ clear_event_triggers(struct trace_array *tr)
>
> list_for_each_entry(file, &tr->events, list) {
> struct event_trigger_data *data, *n;
> +
Again, if it's in a code block, don't change it.
-- Steve
> list_for_each_entry_safe(data, n, &file->triggers, list) {
> trace_event_trigger_enable_disable(file, 0);
> list_del_rcu(&data->list);
> @@ -785,7 +788,7 @@ static void unregister_trigger(char *glob,
> * cmd - the trigger command name
> * glob - the trigger command name optionally prefaced with '!'
> * param_and_filter - text following cmd and ':'
> - * param - text following cmd and ':' and stripped of filter
> + * param - text following cmd and ':' and filter removed
> * filter - the optional filter text following (and including) 'if'
> *
> * To illustrate the use of these components, here are some concrete
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4] kernel/trace: fixed static warnings
2026-04-08 20:24 ` Steven Rostedt
@ 2026-04-09 12:07 ` Abhijith Sriram
0 siblings, 0 replies; 3+ messages in thread
From: Abhijith Sriram @ 2026-04-09 12:07 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mathieu Desnoyers, open list:TRACING,
open list:TRACING
On Wed, Apr 8, 2026 at 10:23 PM Steven Rostedt <rostedt@goodmis.org> wrote:
>
>
> Subject should be:
>
> tracing: Fixed static checker warnings
>
> On Mon, 6 Apr 2026 09:28:34 +0200
> abhijithsriram95@gmail.com wrote:
>
> > From: Abhijith Sriram <abhijithsriram95@gmail.com>
> >
> > The change in the function argument description
> > was due to the static code checker script reading
> > the word filter back to back
> >
>
> The below changes should be beneath the '---'
>
> > Changes in v2:
>
> The last change should be first. In fact, I only care about the last change
> as the previous versions should have the description of what changed.
>
> > - corrected *m = file->private_data to m = file->private_data
> >
> > Changes in v3:
> > - reverted the changes for struct seq_file *m and
> > added a new empty line instead
> >
> > Changes in v4:
>
> That said, this should really be:
>
> Changes since v3: https://lore.kernel.org/all/20260406060046.223496-2-abhijithsriram95@gmail.com/
>
>
> > - added a new empty line before char *buf ...
> > previously this line was relocated to avoid the
> > static check warning.
> >
> > Signed-off-by: Abhijith Sriram <abhijithsriram95@gmail.com>
> > ---
> > kernel/trace/trace_events_trigger.c | 7 +++++--
> > 1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> > index 655db2e82513..664283bcd9ea 100644
> > --- a/kernel/trace/trace_events_trigger.c
> > +++ b/kernel/trace/trace_events_trigger.c
> > @@ -246,7 +246,7 @@ event_triggers_post_call(struct trace_event_file *file,
> > }
> > EXPORT_SYMBOL_GPL(event_triggers_post_call);
> >
> > -#define SHOW_AVAILABLE_TRIGGERS (void *)(1UL)
> > +#define SHOW_AVAILABLE_TRIGGERS ((void *)(1UL))
> >
> > static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
> > {
> > @@ -352,6 +352,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
> > ret = seq_open(file, &event_triggers_seq_ops);
> > if (!ret) {
> > struct seq_file *m = file->private_data;
> > +
>
> This blank line makes the code look worse. Yes, we usually want a blank
> line between the variable declarations and the code, but when it comes to
> code blocks (not functions) that rule is not as strict.
>
> Get rid of this newline.
>
>
> > m->private = file;
> > }
> > }
> > @@ -390,6 +391,7 @@ static ssize_t event_trigger_regex_write(struct file *file,
> > {
> > struct trace_event_file *event_file;
> > ssize_t ret;
> > +
> > char *buf __free(kfree) = NULL;
>
> The char *buf is a declaration. It no new line is expected before it.
>
> >
> > if (!cnt)
> > @@ -633,6 +635,7 @@ clear_event_triggers(struct trace_array *tr)
> >
> > list_for_each_entry(file, &tr->events, list) {
> > struct event_trigger_data *data, *n;
> > +
>
> Again, if it's in a code block, don't change it.
>
> -- Steve
>
> > list_for_each_entry_safe(data, n, &file->triggers, list) {
> > trace_event_trigger_enable_disable(file, 0);
> > list_del_rcu(&data->list);
> > @@ -785,7 +788,7 @@ static void unregister_trigger(char *glob,
> > * cmd - the trigger command name
> > * glob - the trigger command name optionally prefaced with '!'
> > * param_and_filter - text following cmd and ':'
> > - * param - text following cmd and ':' and stripped of filter
> > + * param - text following cmd and ':' and filter removed
> > * filter - the optional filter text following (and including) 'if'
> > *
> > * To illustrate the use of these components, here are some concrete
>
Shall we totally scrap these changes? Apart from introducing new lines
the only other
changes are to add some brackets and reframe the comments. We are not adding
a lot of value from this change. I will take this as a positive first
time experience
into linux kernel dev and focus on other meaningful changes.
--
Regards
Abhijith Sriram
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-09 12:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-06 7:28 [PATCH v4] kernel/trace: fixed static warnings abhijithsriram95
2026-04-08 20:24 ` Steven Rostedt
2026-04-09 12:07 ` Abhijith Sriram
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox