| Message ID | 20200702185703.946652691@goodmis.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools lib traceevent: Patches from the trace-cmd repo | expand |
Hi Steve and Tzvetomir, On Fri, Jul 3, 2020 at 3:57 AM Steven Rostedt <rostedt@goodmis.org> wrote: > > From: "Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> > > Add the API function tep_load_plugins_hook() to the traceevent API to allow > tools a common method to load in the plugins that are part of the lib > traceevent library. > > Link: http://lore.kernel.org/linux-trace-devel/20190802110101.14759-4-tz.stoyanov@gmail.com > Link: http://lore.kernel.org/linux-trace-devel/20200625100516.365338-4-tz.stoyanov@gmail.com > > Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com> > Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> > --- > tools/lib/traceevent/event-parse.h | 6 ++++++ > tools/lib/traceevent/event-plugin.c | 19 +++++++++---------- > 2 files changed, 15 insertions(+), 10 deletions(-) > > diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h > index b77837f75a0d..776c7c24ee79 100644 > --- a/tools/lib/traceevent/event-parse.h > +++ b/tools/lib/traceevent/event-parse.h > @@ -396,6 +396,12 @@ struct tep_plugin_list; > struct tep_plugin_list *tep_load_plugins(struct tep_handle *tep); > void tep_unload_plugins(struct tep_plugin_list *plugin_list, > struct tep_handle *tep); > +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); > char **tep_plugin_list_options(void); > void tep_plugin_free_options_list(char **list); > int tep_plugin_add_options(const char *name, > diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c > index e1f7ddd5a6cf..b53d9a53bcf9 100644 > --- a/tools/lib/traceevent/event-plugin.c > +++ b/tools/lib/traceevent/event-plugin.c > @@ -365,20 +365,19 @@ load_plugins_dir(struct tep_handle *tep, const char *suffix, > closedir(dir); > } > > -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.. > { > 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? Thanks Namhyung > > /* > @@ -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; > } > > -- > 2.26.2 > >
On Tue, 7 Jul 2020 23:55:34 +0900 Namhyung Kim <namhyung@kernel.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; > > }
diff --git a/tools/lib/traceevent/event-parse.h b/tools/lib/traceevent/event-parse.h index b77837f75a0d..776c7c24ee79 100644 --- a/tools/lib/traceevent/event-parse.h +++ b/tools/lib/traceevent/event-parse.h @@ -396,6 +396,12 @@ struct tep_plugin_list; struct tep_plugin_list *tep_load_plugins(struct tep_handle *tep); void tep_unload_plugins(struct tep_plugin_list *plugin_list, struct tep_handle *tep); +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); char **tep_plugin_list_options(void); void tep_plugin_free_options_list(char **list); int tep_plugin_add_options(const char *name, diff --git a/tools/lib/traceevent/event-plugin.c b/tools/lib/traceevent/event-plugin.c index e1f7ddd5a6cf..b53d9a53bcf9 100644 --- a/tools/lib/traceevent/event-plugin.c +++ b/tools/lib/traceevent/event-plugin.c @@ -365,20 +365,19 @@ load_plugins_dir(struct tep_handle *tep, const char *suffix, closedir(dir); } -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) { char *home; char *path; char *envdir; int ret; - if (tep->flags & TEP_DISABLE_PLUGINS) + if (tep && tep->flags & TEP_DISABLE_PLUGINS) return; /* @@ -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; }