[pulseaudio-discuss] [PATCH] module: Assign the index before calling init()

[David Henningsson]

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