[PATCH] dbus: Raise PropertiesChanged on org.freedesktop.DBus.Properties

[Dan Williams]

On Wed, 2011-08-10 at 17:52 +0100, Daniel Drake wrote:
> wpa_supplicant dbus objects are currently difficult to use with
> GDBusProxy. This is because they do not follow the dbus standard in
> emitting the PropertiesChanged signal on the standard D-Bus properties
> interface, so GDBusProxy stores stale property values.
> 
> Fix this by raising PropertiesChanged on the standard interface.
> For backwards compatibility, the existing custom PropertiesChanged
> signals are not removed; I anticipate them being removed in a future
> major release.
> 
> See
> http://lists.freedesktop.org/archives/dbus/2011-August/014593.html
> for more info.
> 
> Signed-off-by: Daniel Drake <dsd at laptop.org>

Looks good to me...

Dan

> ---
>  wpa_supplicant/dbus/dbus_new.c         |    5 ++
>  wpa_supplicant/dbus/dbus_new_helpers.c |   79 ++++++++++++++++++++++++++++++--
>  2 files changed, 80 insertions(+), 4 deletions(-)
> 
> diff --git a/wpa_supplicant/dbus/dbus_new.c b/wpa_supplicant/dbus/dbus_new.c
> index e488937..1f56aba 100644
> --- a/wpa_supplicant/dbus/dbus_new.c
> +++ b/wpa_supplicant/dbus/dbus_new.c
> @@ -1697,6 +1697,7 @@ static const struct wpa_dbus_signal_desc wpas_dbus_global_signals[] = {
>  		  END_ARGS
>  	  }
>  	},
> +	/* Deprecated - use org.freedesktop.DBus.Properties.PropertiesChanged */
>  	{ "PropertiesChanged", WPAS_DBUS_NEW_INTERFACE,
>  	  {
>  		  { "properties", "a{sv}", ARG_OUT },
> @@ -1785,6 +1786,7 @@ static const struct wpa_dbus_property_desc wpas_dbus_network_properties[] = {
>  
> 
>  static const struct wpa_dbus_signal_desc wpas_dbus_network_signals[] = {
> +	/* Deprecated - use org.freedesktop.DBus.Properties.PropertiesChanged */
>  	{ "PropertiesChanged", WPAS_DBUS_NEW_IFACE_NETWORK,
>  	  {
>  		  { "properties", "a{sv}", ARG_OUT },
> @@ -1962,6 +1964,7 @@ static const struct wpa_dbus_property_desc wpas_dbus_bss_properties[] = {
>  
> 
>  static const struct wpa_dbus_signal_desc wpas_dbus_bss_signals[] = {
> +	/* Deprecated - use org.freedesktop.DBus.Properties.PropertiesChanged */
>  	{ "PropertiesChanged", WPAS_DBUS_NEW_IFACE_BSS,
>  	  {
>  		  { "properties", "a{sv}", ARG_OUT },
> @@ -2486,6 +2489,7 @@ static const struct wpa_dbus_signal_desc wpas_dbus_interface_signals[] = {
>  		  END_ARGS
>  	  }
>  	},
> +	/* Deprecated - use org.freedesktop.DBus.Properties.PropertiesChanged */
>  	{ "PropertiesChanged", WPAS_DBUS_NEW_IFACE_INTERFACE,
>  	  {
>  		  { "properties", "a{sv}", ARG_OUT },
> @@ -2506,6 +2510,7 @@ static const struct wpa_dbus_signal_desc wpas_dbus_interface_signals[] = {
>  		  END_ARGS
>  	  }
>  	},
> +	/* Deprecated - use org.freedesktop.DBus.Properties.PropertiesChanged */
>  	{ "PropertiesChanged", WPAS_DBUS_NEW_IFACE_WPS,
>  	  {
>  		  { "properties", "a{sv}", ARG_OUT },
> diff --git a/wpa_supplicant/dbus/dbus_new_helpers.c b/wpa_supplicant/dbus/dbus_new_helpers.c
> index 342ad2d..4c1db88 100644
> --- a/wpa_supplicant/dbus/dbus_new_helpers.c
> +++ b/wpa_supplicant/dbus/dbus_new_helpers.c
> @@ -610,7 +610,7 @@ int wpa_dbus_unregister_object_per_iface(
>  
>  static dbus_bool_t put_changed_properties(
>  	const struct wpa_dbus_object_desc *obj_dsc, const char *interface,
> -	DBusMessageIter *dict_iter)
> +	DBusMessageIter *dict_iter, int clear_changed)
>  {
>  	DBusMessageIter entry_iter;
>  	const struct wpa_dbus_property_desc *dsc;
> @@ -624,7 +624,8 @@ static dbus_bool_t put_changed_properties(
>  			continue;
>  		if (os_strcmp(dsc->dbus_interface, interface) != 0)
>  			continue;
> -		obj_dsc->prop_changed_flags[i] = 0;
> +		if (clear_changed)
> +			obj_dsc->prop_changed_flags[i] = 0;
>  
>  		if (!dbus_message_iter_open_container(dict_iter,
>  						      DBUS_TYPE_DICT_ENTRY,
> @@ -660,7 +661,56 @@ static dbus_bool_t put_changed_properties(
>  }
>  
> 
> -static void send_prop_changed_signal(
> +static void do_send_prop_changed_signal(
> +	DBusConnection *con, const char *path, const char *interface,
> +	const struct wpa_dbus_object_desc *obj_dsc)
> +{
> +	DBusMessage *msg;
> +	DBusMessageIter signal_iter, dict_iter;
> +
> +	msg = dbus_message_new_signal(path, DBUS_INTERFACE_PROPERTIES,
> +				      "PropertiesChanged");
> +	if (msg == NULL)
> +		return;
> +
> +	dbus_message_iter_init_append(msg, &signal_iter);
> +
> +	if (!dbus_message_iter_append_basic(&signal_iter, DBUS_TYPE_STRING,
> +					    &interface))
> +		goto err;
> +
> +	/* Changed properties dict */
> +	if (!dbus_message_iter_open_container(&signal_iter, DBUS_TYPE_ARRAY,
> +					      "{sv}", &dict_iter))
> +		goto err;
> +
> +	if (!put_changed_properties(obj_dsc, interface, &dict_iter, 0))
> +		goto err;
> +
> +	if (!dbus_message_iter_close_container(&signal_iter, &dict_iter))
> +		goto err;
> +
> +	/* Invalidated properties array (empty) */
> +	if (!dbus_message_iter_open_container(&signal_iter, DBUS_TYPE_ARRAY,
> +					      "s", &dict_iter))
> +		goto err;
> +
> +	if (!dbus_message_iter_close_container(&signal_iter, &dict_iter))
> +		goto err;
> +
> +	dbus_connection_send(con, msg, NULL);
> +
> +out:
> +	dbus_message_unref(msg);
> +	return;
> +
> +err:
> +	wpa_printf(MSG_DEBUG, "dbus: %s: Failed to construct signal",
> +		   __func__);
> +	goto out;
> +}
> +
> +static void do_send_deprecated_prop_changed_signal(
>  	DBusConnection *con, const char *path, const char *interface,
>  	const struct wpa_dbus_object_desc *obj_dsc)
>  {
> @@ -677,7 +727,7 @@ static void send_prop_changed_signal(
>  					      "{sv}", &dict_iter))
>  		goto err;
>  
> -	if (!put_changed_properties(obj_dsc, interface, &dict_iter))
> +	if (!put_changed_properties(obj_dsc, interface, &dict_iter, 1))
>  		goto err;
>  
>  	if (!dbus_message_iter_close_container(&signal_iter, &dict_iter))
> @@ -695,6 +745,27 @@ err:
>  	goto out;
>  }
>  
> +static void send_prop_changed_signal(
> +	DBusConnection *con, const char *path, const char *interface,
> +	const struct wpa_dbus_object_desc *obj_dsc)
> +{
> +	/*
> +	 * First, send property change notification on the standardized
> +	 * org.freedesktop.DBus.Properties interface. This call will not
> +	 * clear the property change bits, so that they are preserved for
> +	 * the call that follows.
> +	 */
> +	do_send_prop_changed_signal(con, path, interface, obj_dsc);
> +
> +	/*
> +	 * Now send PropertiesChanged on our own interface for backwards
> +	 * compatibility. This is deprecated and will be removed in a future
> +	 * release.
> +	 */
> +	do_send_deprecated_prop_changed_signal(con, path, interface, obj_dsc);
> +
> +	/* Property change bits have now been cleared. */
> +}
>  
>  static void flush_object_timeout_handler(void *eloop_ctx, void *timeout_ctx)
>  {