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

[Abhijith Sriram]

On Mon, Apr 6, 2026 at 4:00 AM Masami Hiramatsu <mhiramat@xxxxxxxxxx> wrote:
>
> 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.
I reverted the changes in version 3 of the patch.
Please find it here:
https://lore.kernel.org/linux-trace-kernel/20260406060046.223496-2-abhijithsriram95@xxxxxxxxx/T/#u
>
> > >
> > > >                       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>

Regards
Abhijith Sriram