On 04/29/2013 03:46 PM, Tanu Kaskinen wrote: > Any code that runs inside the init() callback sees an invalid module > index. Sometimes init() does things that cause hooks to be fired. This > means that any code that uses hooks may see an invalid module index. > Fix this by assigning the module index before init() is called. > > There are no known issues in the upstream code base where an invalid > module index would be used, but an out-of-tree module > (module-murphy-ivi) had a problem with this. I guess there could be a point that the module is not seen by the core before it is completely loaded, but I don't mind either way. If it helps you, go ahead (after fixing the comment below). > > BugLink: https://bugs.freedesktop.org/show_bug.cgi?id=63923 > --- > src/pulsecore/module.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/src/pulsecore/module.c b/src/pulsecore/module.c > index 6f276bb..f30a3ce 100644 > --- a/src/pulsecore/module.c > +++ b/src/pulsecore/module.c > @@ -113,14 +113,14 @@ pa_module* pa_module_load(pa_core *c, const char *name, const char *argument) { > m->core = c; > m->unload_requested = FALSE; > > + pa_assert_se(pa_idxset_put(c->modules, m, &m->index) >= 0); > + pa_assert(m->index != PA_IDXSET_INVALID); > + > if (m->init(m) < 0) { > pa_log_error("Failed to load module \"%s\" (argument: \"%s\"): initialization failed.", name, argument ? argument : ""); > goto fail; > } > > - pa_assert_se(pa_idxset_put(c->modules, m, &m->index) >= 0); > - pa_assert(m->index != PA_IDXSET_INVALID); > - > pa_log_info("Loaded \"%s\" (index: #%u; argument: \"%s\").", m->name, m->index, m->argument ? m->argument : ""); > > pa_subscription_post(c, PA_SUBSCRIPTION_EVENT_MODULE|PA_SUBSCRIPTION_EVENT_NEW, m->index); > @@ -144,6 +144,9 @@ pa_module* pa_module_load(pa_core *c, const char *name, const char *argument) { > fail: > > if (m) { > + if (m->index != PA_IDXSET_INVALID) > + pa_idxset_remove_by_index(c->modules, m->index); m->index must be initialized to PA_IDXSET_INVALID, for jumps to "fail" before pa_idxset_put to behave correctly. > + > if (m->proplist) > pa_proplist_free(m->proplist); > > -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic
