[Bug 40380] Add tp_dbus_properties_mixin_emit_properties_changed

[bugzilla-daemon at freedesktop.org]

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

--- Comment #2 from Will Thompson <will.thompson at collabora.co.uk> 2011-08-26 03:35:56 PDT ---
(In reply to comment #1)
> Just a few bikeshedding points:
> 
> + g_critical ("Couldn't fetch '%s' on interface '%s': %s",
> +     property, iface, error->message);
> 
> I can't remember whether g_critical will include the function name in the log
> message but perhaps have "Couldn't fetch property '%s..." to make it clearer?

I should be using CRITICAL, which does include the function name

> + g_return_val_if_fail (self != NULL, FALSE);
> + g_return_val_if_fail (G_IS_OBJECT (self), FALSE);
> 
> Not sure both of these are necessary?

One of the type-checking macros returns TRUE for NULL. I can never remember
which one. I *think* it's _IS_.

> -   g_value_init (value, prop_info->type);
> -   iface_impl->getter (self, iface_info->dbus_interface,
> -       prop_info->name, value, prop_impl->getter_data);
> +   if (value != NULL)
> +     {
> +       g_value_init (value, prop_info->type);
> +       iface_impl->getter (self, iface_info->dbus_interface,
> +       prop_info->name, value, prop_impl->getter_data);
> +     }
> 
> Is this meant to be in the topmost commit?

Yes—emit_properties_changed calls _iface_impl_get_property with a NULL GValue
if it doesn't actually want the value.

> + tp_dbus_properties_mixin_emit_properties_changed (
> +     GObject *object,
> +     const gchar *interface_name,
> +     const gchar * const *changed,
> +     const gchar * const *invalidated)
> 
> Not sure about only passing the names of the changed properties. It's only a
> bit crap if getting the property is expensive or time-consuming, but I guess
> that never happens, right? Yeah, probably not, carry on.

I don't understand the potential complaint here: did you mean “why don't we
just always include all the properties in the interface on every signal”?

> + WARNING ("u mad? %s", error->message);
> 
> soz.

Yeah yeah.

-- 
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.