Bug 29547

http://git.collabora.co.uk/?p=user/danni/telepathy-glib.git;a=shortlog;h=refs/heads/tp-asv-copy-29547

> +  /* 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).

Typing it out in full makes it explicit, but also requires you to get it right. I can add more documentation to try and clarify things.

In hindsight, maybe we should have allocated the keys as well?

Danni's dead bug closing spree.