[pulseaudio-discuss] [PATCH v0 02/11] card: Support adding ports dynamically

[Tanu Kaskinen]

On Mon, 2012-10-22 at 10:46 +0200, Mikel Astiz wrote:
> From: Mikel Astiz <mikel.astiz at bmw-carit.de>
> 
> Card profiles -specially the ones registered with pa_card_add_profile()-
> might need to create new ports during the lifetime of the card.
> ---
>  src/pulsecore/card.c | 25 +++++++++++++++++++++++++
>  src/pulsecore/card.h |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/src/pulsecore/card.c b/src/pulsecore/card.c
> index 79fe41c..25128eb 100644
> --- a/src/pulsecore/card.c
> +++ b/src/pulsecore/card.c
> @@ -94,6 +94,31 @@ int pa_card_add_profile(pa_card *c, struct pa_card_profile *profile) {
>      return 0;
>  }
>  
> +int pa_card_add_ports(pa_card *c, pa_hashmap *ports) {
> +    pa_device_port *p;
> +    void *state;
> +
> +    pa_assert(c);
> +    pa_assert(ports);
> +
> +    PA_HASHMAP_FOREACH(p, ports, state)
> +        if (pa_hashmap_get(c->ports, p->name)) {
> +            pa_log_error("Card %s already has port %s", c->name, p->name);
> +            return -1;
> +        }

Same thing as with the previous patch: instead of returning an error,
you could just have an assertion that pa_hashmap_put() succeeds.

> +
> +    /* take ownership of the ports */
> +    PA_HASHMAP_FOREACH(p, ports, state)
> +        pa_hashmap_put(c->ports, p->name, p);
> +
> +    while ((p = pa_hashmap_steal_first(ports)) != NULL)
> +        pa_hook_fire(&c->core->hooks[PA_CORE_HOOK_PORT_AVAILABLE_CHANGED], p);

I don't like overloading hooks to mean multiple things. I'd just create
a new hook for port add events.

> +
> +    pa_device_port_hashmap_free(ports);

While this may save one line of code per call site, I think freeing the
hashmap here is also a very unobvious thing, so when reading the calling
code, I might think that there's a memory leak since the hashmap never
gets freed. I would prefer leaving it as a responsibility of the caller
to free the hashmap.

-- 
Tanu