linux-kernel - Re: [PATCH v2 03/15] tools lib traceevent: Add tep_load_plugins

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]

Date:   Tue, 7 Jul 2020 11:35:39 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Namhyung Kim <namhyung@...nel.org>
Cc:     linux-kernel <linux-kernel@...r.kernel.org>,
        Linux Trace Devel <linux-trace-devel@...r.kernel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Ingo Molnar <mingo@...nel.org>, Jiri Olsa <jolsa@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@...il.com>
Subject: Re: [PATCH v2 03/15] tools lib traceevent: Add
 tep_load_plugins_hook() API

On Tue, 7 Jul 2020 23:55:34 +0900
Namhyung Kim <namhyung@...nel.org> wrote:

> > -static void
> > -load_plugins(struct tep_handle *tep, const char *suffix,
> > -            void (*load_plugin)(struct tep_handle *tep,
> > -                                const char *path,
> > -                                const char *name,
> > -                                void *data),
> > -            void *data)
> > +void tep_load_plugins_hook(struct tep_handle *tep, const char *suffix,
> > +                          void (*load_plugin)(struct tep_handle *tep,
> > +                                              const char *path,
> > +                                              const char *name,
> > +                                              void *data),
> > +                          void *data)  
> 
> Can we have a comment (or a doc) for this API?  I'm not sure how it's used..

Actually, this requires a man page.

> 
> 
> >  {
> >         char *home;
> >         char *path;
> >         char *envdir;
> >         int ret;
> >
> > -       if (tep->flags & TEP_DISABLE_PLUGINS)
> > +       if (tep && tep->flags & TEP_DISABLE_PLUGINS)
> >                 return;  
> 
> Is it ok to call with a NULL tep handle?

Hmm, it looks like we could possibly pass this in without a tep handle,
if the plugins don't need a it. I'm not sure we have a use case for
that. I'll need to look deeper at this.

Thanks for the review!

-- Steve

> >
> >         /*
> > @@ -386,7 +385,7 @@ load_plugins(struct tep_handle *tep, const char
> > *suffix,
> >          * check that first.
> >          */
> >  #ifdef PLUGIN_DIR
> > -       if (!(tep->flags & TEP_DISABLE_SYS_PLUGINS))
> > +       if (!tep || !(tep->flags & TEP_DISABLE_SYS_PLUGINS))
> >                 load_plugins_dir(tep, suffix, PLUGIN_DIR,
> >                                  load_plugin, data);
> >  #endif
> > @@ -423,7 +422,7 @@ tep_load_plugins(struct tep_handle *tep)
> >  {
> >         struct tep_plugin_list *list = NULL;
> >
> > -       load_plugins(tep, ".so", load_plugin, &list);
> > +       tep_load_plugins_hook(tep, ".so", load_plugin, &list);
> >         return list;
> >  }