[Bug 77194] [next] avoid dbus-glib type for TpBaseChannelFillPropertiesFunc

[bugzilla-daemon at freedesktop.org]

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

Simon McVittie <simon.mcvittie at collabora.co.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Depends on|                            |77189
             Blocks|                            |77196

--- Comment #1 from Simon McVittie <simon.mcvittie at collabora.co.uk> ---
Xavier's gdbus-all branch fixes this, although it needs rebasing onto my branch
from Bug #77189, and all the CMs need to catch up.

Review, ignoring the two trivial gtk-doc commits that I cherry-picked to my
gdbus-object branch in Bug #77189:

----

5d0ab7f2: looks fine

----

805dfb02:

It isn't necessarily a huge problem that this lacks a regression test in
telepathy-glib, because Idle and Gabble should be testing it.

+static void
+channel_died_cb (gpointer data,
+    GObject *deceased_channel)

How can this happen, now that we take a strong ref to the channel? (I think
that strong ref is probably a mistake - isn't it circular?)

----

1460a071:

This is really Bug #77197 and I don't see how it relates to channels at all
really, so I'll review it over there.

----

e664dd8b:

+  if (klass->fill_immutable_properties)
+    klass->fill_immutable_properties (chan, &properties);

No need for the conditional, the base class implements it. Not a blocker.

This commit does part of Bug #77197 too - it makes TpBaseProtocol's immutable
properties GVariant-based. I can see that you wanted to avoid having to have
both GHashTable and GVariantDict versions of
tp_dbus_properties_mixin_fill_properties_hash(), but it still seems odd to me.
Still, we want to do both eventually...

+          gchar *key = g_strdup_printf ("%s.%s", iface, property);
+
+          g_variant_dict_insert_value (dict, key,
+              dbus_g_value_build_g_variant (&value));
+          g_value_unset (&value);

key is leaked

> tp_dbus_properties_mixin_fill_properties_hash

This should have a different name now. No hash table is involved.
fill_properties_dict?

----

General:

I think we should make TpBaseChannel and TpBasePasswordChannel use a non-TpSvc*
skeleton on this bug too (or clone a bug), to keep all the channel stuff
vaguely together. It can be a follow-up branch if that's easier.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
You are the assignee for the bug.