[PATCH] dbus: Handle optional properties in PropertiesChanged

[PATCH] dbus: Handle optional properties in PropertiesChanged

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 2852 bytes --]

Handle property getter returning an error when building the
PropertiesChanged signal instead of returning NULL.  This is important
because the signal may include multiple properties and if one getter
fails other properties should still be included.  One options is to only
error out if all getters fail, another is to add them to the invalidated
properties list of the signal.  The latter seems to be the right thing
to do for optional properties that may appear and disappear.
---
 ell/dbus-service.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index deffc07..57cafbe 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -962,6 +962,7 @@ static struct l_dbus_message *build_properties_changed_signal(
 	const struct l_queue_entry *entry;
 	const struct _dbus_property *property;
 	const char *signature;
+	struct l_queue *invalidated;
 
 	signal = l_dbus_message_new_signal(dbus, rec->path,
 						L_DBUS_INTERFACE_PROPERTIES,
@@ -969,6 +970,8 @@ static struct l_dbus_message *build_properties_changed_signal(
 
 	builder = l_dbus_message_builder_new(signal);
 
+	invalidated = l_queue_new();
+
 	l_dbus_message_builder_append_basic(builder, 's',
 						rec->instance->interface->name);
 	l_dbus_message_builder_enter_array(builder, "{sv}");
@@ -978,6 +981,8 @@ static struct l_dbus_message *build_properties_changed_signal(
 		property = entry->data;
 		signature = property->metainfo + strlen(property->metainfo) + 1;
 
+		_dbus_message_builder_mark(builder);
+
 		l_dbus_message_builder_enter_dict(builder, "sv");
 		l_dbus_message_builder_append_basic(builder, 's',
 							property->metainfo);
@@ -985,10 +990,16 @@ static struct l_dbus_message *build_properties_changed_signal(
 
 		if (!property->getter(dbus, signal, builder,
 					rec->instance->user_data)) {
-			l_dbus_message_builder_destroy(builder);
-			l_dbus_message_unref(signal);
+			if (!_dbus_message_builder_rewind(builder)) {
+				l_dbus_message_unref(signal);
+				signal = NULL;
 
-			return NULL;
+				goto done;
+			}
+
+			l_queue_push_tail(invalidated, (void *) property);
+
+			continue;
 		}
 
 		l_dbus_message_builder_leave_variant(builder);
@@ -997,10 +1008,19 @@ static struct l_dbus_message *build_properties_changed_signal(
 
 	l_dbus_message_builder_leave_array(builder);
 	l_dbus_message_builder_enter_array(builder, "s");
+
+	while ((property = l_queue_pop_head(invalidated)))
+		l_dbus_message_builder_append_basic(builder, 's',
+							property->metainfo);
+
 	l_dbus_message_builder_leave_array(builder);
 	l_dbus_message_builder_finalize(builder);
+
+done:
 	l_dbus_message_builder_destroy(builder);
 
+	l_queue_destroy(invalidated, NULL);
+
 	return signal;
 }
 
-- 
2.7.4

Re: [PATCH] dbus: Handle optional properties in PropertiesChanged

[Denis Kenzior]

[-- Attachment #1: Type: text/plain, Size: 725 bytes --]

Hi Andrew,

On 09/21/2016 11:41 AM, Andrew Zaborowski wrote:
> Handle property getter returning an error when building the
> PropertiesChanged signal instead of returning NULL.  This is important
> because the signal may include multiple properties and if one getter
> fails other properties should still be included.  One options is to only
> error out if all getters fail, another is to add them to the invalidated
> properties list of the signal.  The latter seems to be the right thing
> to do for optional properties that may appear and disappear.
> ---
>   ell/dbus-service.c | 26 +++++++++++++++++++++++---
>   1 file changed, 23 insertions(+), 3 deletions(-)
>

Applied, thanks.

Regards,
-Denis