[pulseaudio-discuss] [PATCH 13/13] Remove refcounting from ports, sinks and sources

[Tanu Kaskinen]

On Thu, 2013-02-14 at 15:02 +0100, David Henningsson wrote:
> On 02/12/2013 08:37 PM, Tanu Kaskinen wrote:
> > since I dislike refcounting, I decided to spend
> > the effort on removing the refcounting from ports instead. That
> > required removing it also from sinks and sources.
> >
> > Ports are now owned solely by cards, except if a sink or source
> > doesn't belong to a card, in which case the sink or source owns its
> > ports.
> 
> I chose refcounting for this reason: It is sometimes being owned by 
> sinks/sources, and sometimes by cards. Therefore it seemed convenient to 
> have refcounting instead of the above scheme; which seems more error prone.
> 
> But, how about instead having an "void* owner" pointer in the port? Then 
> sink_free could just to "if (port->owner == me) port_free(port);" That 
> seems slightly less error prone to me.

I think it's also easier to read. I'll do this change.

> Even if I would say that *this* problem is more about the hashmap: It 
> would be better to have a string-key-hashmap type that owned the memory 
> of the key strings rather than the current scheme, which cause quite 
> subtle errors, such as the one you're currently trying to correct.

I disagree that string-key-hashmap would be the right solution for
*this* problem. The hashmap would still contain stale profile pointers.
If the profile pointers are never dereferenced after the profiles have
been freed, then that wouldn't cause any visible problems, but I think
it's still wrong to store stale pointers, as a principle.

There would probably still be use for a string-key-hashmap class. Or
perhaps the best approach would be to have a hashmap constructor similar
to what GLib uses:

GHashTable *g_hash_table_new_full(GHashFunc hash_func,
                                  GEqualFunc key_equal_func,
                                  GDestroyNotify key_destroy_func,
                                  GDestroyNotify value_destroy_func);

That is suitable for the suggested "string-key-hashmap" use case and
also for the current convention of not having a separate free function
for the keys. I quite like this approach, so if this gets support, I
volunteer to implement this.

-- 
Tanu