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
