[Bug 20942] tp_asv_new() and tp_asv_set_*

bugzilla-daemon at freedesktop.org bugzilla-daemon at freedesktop.org
Wed Apr 1 15:48:43 CEST 2009


http://bugs.freedesktop.org/show_bug.cgi?id=20942





--- Comment #4 from Simon McVittie <simon.mcvittie at collabora.co.uk>  2009-04-01 06:48:43 PST ---
Some review comments:

Each tp_asv_set_* that takes a pointer should have tp_asv_take_* and
tp_asv_set_static_* variants. The _take_ ones are useful when you're allocating
something on the spot, and the _set_static_ ones are useful when you have some
immutable static data in your C file.

I think tp_asv_set_bytes() should be slightly inconsistent by taking a length
and a gconstpointer, like tp_g_value_slice_new_bytes() does, so it can be used
for non-GArray sources of bytes (since the byte array is being copied anyway,
there's no requirement that they come from a genuine GArray). If what you're
copying *is* in fact a GArray, then tp_asv_set_bytes (asv, "BinaryBlob",
array->len, array->data) is just as easy.

(You might have missed some of the boxed GValue slice allocators, because
they're in dbus.[ch] rather than util.[ch] - the idea is that anything using
GLib can benefit from sliced GValues, but the boxed ones like object path and
bytes are specific to dbus-glib).

tp_asv_new() should have the G_GNUC_NULL_TERMINATED annotation.


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



More information about the telepathy-bugs mailing list