Re: [PATCH] kernel/trace: fixed static warnings

[Google]

On Sat, 4 Apr 2026 08:03:07 +0200
Abhijith Sriram <abhijithsriram95@xxxxxxxxx> wrote:

> On Sat, Apr 4, 2026 at 2:18 AM Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
> >
> > On Thu,  2 Apr 2026 21:54:04 +0200
> > abhijithsriram95@xxxxxxxxx wrote:
> >
> > > From: Abhijith Sriram <abhijithsriram95@xxxxxxxxx>
> > >
> > > The change in the function argument description
> > > was due to the static code checker script reading
> > > the word filter back to back
> > >
> > > Signed-off-by: Abhijith Sriram <abhijithsriram95@xxxxxxxxx>
> > > ---
> > >  kernel/trace/trace_events_trigger.c | 10 ++++++----
> > >  1 file changed, 6 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
> > > index 655db2e82513..477d8dee3362 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))
> >
> > This is OK.
> >
> > >
> > >  static void *trigger_next(struct seq_file *m, void *t, loff_t *pos)
> > >  {
> > > @@ -325,6 +325,7 @@ static const struct seq_operations event_triggers_seq_ops = {
> > >  static int event_trigger_regex_open(struct inode *inode, struct file *file)
> > >  {
> > >       int ret;
> > > +     struct seq_file *m = NULL;
> > >
> > >       ret = security_locked_down(LOCKDOWN_TRACEFS);
> > >       if (ret)
> > > @@ -351,7 +352,7 @@ static int event_trigger_regex_open(struct inode *inode, struct file *file)
> > >       if (file->f_mode & FMODE_READ) {
> > >               ret = seq_open(file, &event_triggers_seq_ops);
> > >               if (!ret) {
> > > -                     struct seq_file *m = file->private_data;
> > > +                     *m = file->private_data;
> >
> > Why is this change required?
> The original warning says "missing blank line after declaration". I
> thought it was cleaner to have the
> declaration in the beginning of the function. I made a mistake here
> which I fixed in the version 2 of
> the patch, please have a look here:
> https://lore.kernel.org/linux-trace-kernel/20260403071108.23422-2-abhijithsriram95@xxxxxxxxx/T/#u

In that case, you just need to add an empty line, no need to move the
definition becuase it changes the scope of `m` variable.

> >
> > >                       m->private = file;
> > >               }
> > >       }
> > > @@ -388,9 +389,9 @@ static ssize_t event_trigger_regex_write(struct file *file,
> > >                                        const char __user *ubuf,
> > >                                        size_t cnt, loff_t *ppos)
> > >  {
> > > +     char *buf __free(kfree) = NULL;
> > >       struct trace_event_file *event_file;
> > >       ssize_t ret;
> > > -     char *buf __free(kfree) = NULL;
> >
> > What is this change?
> The same missing blank lines after declaration was triggered here,
> even though there is a blank line after the char *buf.
> If I do give an empty line then there is another error "Trailing white
> space". So I  reordered it and the warning disappeared.
> This change I am not super sure since it is usually recommended that
> variables of larger size are declared first
> for padding purposes. What do you think?

Ah, that is a known checkpatch's bug. It does not understand __free()
macro. So please ignore that error (or/and fix checkpatch.pl).

Thanks,

> >
> > Thanks,
> >
> > >
> > >       if (!cnt)
> > >               return 0;
> > > @@ -633,6 +634,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 +787,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
> > >
> >
> >
> > --
> > Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> -- 
> Regards
> Abhijith Sriram


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>