[PATCH weston 4/4] logind: fix PropertiesChanged parser

[nerdopolis]

On Tuesday, December 30, 2014 02:33:23 PM David Herrmann wrote:
> The current parser directly reads a BOOLEAN on the PropertiesChanged
> signal for 'Active' properties. However, all property-values are packed in
> a VARIANT, otherwise, we wouldn't know the type. Fix the parser to recurse
> into the variant before reading the boolean.
> 
> To avoid such bugs in the future, we extract the 'Active' parser into a
> helper function parse_active(), which is then shared between the
> PropertiesChanged and Get handlers.
> 
> Signed-off-by: David Herrmann <dh.herrmann at gmail.com>
> ---
>  src/logind-util.c | 56 +++++++++++++++++++++++++++----------------------------
>  1 file changed, 28 insertions(+), 28 deletions(-)
> 
> diff --git a/src/logind-util.c b/src/logind-util.c
> index db23606..e4e20eb 100644
> --- a/src/logind-util.c
> +++ b/src/logind-util.c
> @@ -287,33 +287,18 @@ weston_logind_set_active(struct weston_logind *wl, bool active)
>  }
>  
>  static void
> -get_active_cb(DBusPendingCall *pending, void *data)
> +parse_active(struct weston_logind *wl, DBusMessage *m, DBusMessageIter *iter)
>  {
> -	struct weston_logind *wl = data;
> -	DBusMessage *m;
> -	DBusMessageIter iter, sub;
> -	int type;
> +	DBusMessageIter sub;
>  	dbus_bool_t b;
>  
> -	dbus_pending_call_unref(wl->pending_active);
> -	wl->pending_active = NULL;
> -
> -	m = dbus_pending_call_steal_reply(pending);
> -	if (!m)
> +	if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_VARIANT)
>  		return;
>  
> -	type = dbus_message_get_type(m);
> -	if (type != DBUS_MESSAGE_TYPE_METHOD_RETURN)
> -		goto err_unref;
> -
> -	if (!dbus_message_iter_init(m, &iter) ||
> -	    dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> -		goto err_unref;
> -
> -	dbus_message_iter_recurse(&iter, &sub);
> +	dbus_message_iter_recurse(iter, &sub);
>  
>  	if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
> -		goto err_unref;
> +		return;
>  
>  	dbus_message_iter_get_basic(&sub, &b);
>  
> @@ -322,8 +307,28 @@ get_active_cb(DBusPendingCall *pending, void *data)
>  	 * other backends, we immediately forward the Active-change event. */
>  	if (!wl->sync_drm || !b)
>  		weston_logind_set_active(wl, b);
> +}
> +
> +static void
> +get_active_cb(DBusPendingCall *pending, void *data)
> +{
> +	struct weston_logind *wl = data;
> +	DBusMessageIter iter;
> +	DBusMessage *m;
> +	int type;
> +
> +	dbus_pending_call_unref(wl->pending_active);
> +	wl->pending_active = NULL;
> +
> +	m = dbus_pending_call_steal_reply(pending);
> +	if (!m)
> +		return;
> +
> +	type = dbus_message_get_type(m);
> +	if (type == DBUS_MESSAGE_TYPE_METHOD_RETURN &&
> +	    dbus_message_iter_init(m, &iter))
> +		parse_active(wl, m, &iter);
>  
> -err_unref:
>  	dbus_message_unref(m);
>  }
>  
> @@ -408,7 +413,6 @@ property_changed(struct weston_logind *wl, DBusMessage *m)
>  {
>  	DBusMessageIter iter, sub, entry;
>  	const char *interface, *name;
> -	dbus_bool_t b;
>  
>  	if (!dbus_message_iter_init(m, &iter) ||
>  	    dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> @@ -433,12 +437,8 @@ property_changed(struct weston_logind *wl, DBusMessage *m)
>  			goto error;
>  
>  		if (!strcmp(name, "Active")) {
> -			if (dbus_message_iter_get_arg_type(&entry) == DBUS_TYPE_BOOLEAN) {
> -				dbus_message_iter_get_basic(&entry, &b);
> -				if (!b)
> -					weston_logind_set_active(wl, false);
> -				return;
> -			}
> +			parse_active(wl, m, &entry);
> +			return;
>  		}
>  
>  		dbus_message_iter_next(&sub);
> 
I tested this path series on both framebuffer and DRM backends. I am now able to properly TTY switch with Weston sessions started by logind using the fbdev backend.