Messages in this thread | | | From | Namhyung Kim <> | Subject | Re: [PATCH 01/22] tools lib traceevent: Add plugin support | Date | Mon, 25 Nov 2013 16:33:39 +0900 |
| |
On Sat, 23 Nov 2013 03:12:19 -0500, Steven Rostedt wrote: > On Fri, 22 Nov 2013 23:17:06 +0900 > Namhyung Kim <namhyung@kernel.org> wrote: > >> > >> [SNIP[ >> > +static void >> > +load_plugin(struct pevent *pevent, const char *path, >> > + const char *file, void *data) >> > +{ >> > + struct plugin_list **plugin_list = data; >> > + pevent_plugin_load_func func; >> > + struct plugin_list *list; >> > + const char *alias; >> > + char *plugin; >> > + void *handle; >> > + >> > + plugin = malloc_or_die(strlen(path) + strlen(file) + 2); >> >> I'd like not to see this malloc_or_die() anymore in a new code. Just >> returning after showing a warning looks enough here. > > Yeah I agree. This is a relic from my code. I think it's OK to add > here, as it is pretty much direct port of my code, and then we can just > add a patch against it to remove it.
Okay. I agree that it'd be better to make them separate patches.
> >> >> > + >> > + strcpy(plugin, path); >> > + strcat(plugin, "/"); >> > + strcat(plugin, file); >> > + >> > + handle = dlopen(plugin, RTLD_NOW | RTLD_GLOBAL); >> >> Why RTLD_NOW and RTLD_GLOBAL? Hmm.. maybe using _NOW is needed to >> prevent a runtime error, but not sure why _GLOBAL is needed. > > Yes, we want to make sure all symbols defined are available at time of > load, otherwise bail out. > >> >> IIUC _GLOBAL is for exporting symbols to *other libraries*. Is it >> intended for this plugin support? > > That was the plan. To have one plugin supply a set of functions that > other plugins may use. That is what GLOBAL is for, right? I don't > recall if I every did this, but it was something I wanted for future > work. > > Now if we don't need it, we could remove it, but is it bad to have?
I might be slow down symbol resolution of new plugins tiny bit. But I don't think it's a real problem as its effect will be negligible.
I don't object the code but just want to know your intention. :)
Thanks, Namhyung
| |