[Bug 29547] new api: tp_asv_copy()

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Fri Aug 13 12:10:49 CEST 2010


https://bugs.freedesktop.org/show_bug.cgi?id=29547

--- Comment #2 from Simon McVittie <simon.mcvittie at collabora.co.uk> 2010-08-13 03:10:49 PDT ---
> +  /* keys are not copied (always assumed to be static strings) */

This is why I don't like this API (and also tp_asv_new and the tp_asv_set
family). g_hash_table_new_full() + tp_g_hash_table_update() only takes one more
line, and it makes the memory allocation model explicit.

In practice, Telepathy processes often have to deal with an a{sv} where the
keys are deep-copied too: tp_base_protocol_get_immutable_properties() and
tp_dbus_properties_mixin_fill_properties_hash() work like this. Blindly
assuming that keys are valid forever is going to crash you sooner or later. In
particular, TpAccountChannelRequest got its hash table from the API user, and
there are no guarantees that the API user will be giving us immortal strings.

If you insist on adding tp_asv_copy, its name and documentation should make it
completely clear what its memory allocation model is, but to be honest I'd be
inclined to say "just use tp_g_hash_table_update".

If you want a read-only reference to the request, ref either the
TpAccountChannelRequest or the hash table instead.

With hindsight, I should have insisted that tp_asv_set_foo() worked like
GVariantBuilder, with an opaque object that stores things internally, then
returns a hash table from a self-destruct function (tp_asv_end perhaps).

-- 
Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.



More information about the telepathy-bugs mailing list