[PATCH 1/8] dbus: More complete checks when removing nodes.

[PATCH 1/8] dbus: More complete checks when removing nodes.

[Andrew Zaborowski]

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

This addresses two problems with nodes being freed from the object tree
but not from tree->objects.
---
 ell/dbus-service.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index e082226..639872b 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -660,6 +660,9 @@ void _dbus_object_tree_prune_node(struct object_node *node)
 		if (parent->children != NULL)
 			return;
 
+		if (parent->instances)
+			return;
+
 		node = parent;
 		parent = node->parent;
 	}
@@ -741,9 +744,11 @@ bool _dbus_object_tree_unregister(struct _dbus_object_tree *tree,
 	if (instance)
 		interface_instance_free(instance);
 
-	if (l_queue_isempty(node->instances) && !node->children) {
+	if (l_queue_isempty(node->instances)) {
 		l_hashmap_remove(tree->objects, path);
-		_dbus_object_tree_prune_node(node);
+
+		if (!node->children)
+			_dbus_object_tree_prune_node(node);
 	}
 
 	return r;
-- 
2.5.0

[PATCH 2/8] dbus: setters and getters API for properties.

[Andrew Zaborowski]

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

These setters and getters are not used yet.

(I will split all of the patches by directories when submitting a final
version, this is just for the ease of reviewing)
---
 ell/dbus-service.c       | 28 ++++++------------
 ell/dbus-service.h       | 26 +++++++++++------
 examples/dbus-service.c  | 75 ++++++++++++++++++++++++++++++++++++++++++++++--
 unit/test-dbus-service.c | 20 ++++++++++++-
 4 files changed, 119 insertions(+), 30 deletions(-)

diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 639872b..772e7a7 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -61,6 +61,8 @@ struct _dbus_signal {
 };
 
 struct _dbus_property {
+	l_dbus_property_get_cb_t getter;
+	l_dbus_property_set_cb_t setter;
 	uint32_t flags;
 	unsigned char name_len;
 	char metainfo[];
@@ -191,7 +193,7 @@ void _dbus_property_introspection(struct _dbus_property *info,
 	l_string_append_printf(buf, "\t\t<property name=\"%s\" type=\"%s\" ",
 				info->metainfo, signature);
 
-	if (info->flags & L_DBUS_PROPERTY_FLAG_WRITABLE)
+	if (info->setter)
 		l_string_append(buf, "access=\"readwrite\"");
 	else
 		l_string_append(buf, "access=\"read\"");
@@ -375,7 +377,9 @@ LIB_EXPORT bool l_dbus_interface_signal(struct l_dbus_interface *interface,
 
 LIB_EXPORT bool l_dbus_interface_property(struct l_dbus_interface *interface,
 					const char *name, uint32_t flags,
-					const char *signature)
+					const char *signature,
+					l_dbus_property_get_cb_t getter,
+					l_dbus_property_set_cb_t setter)
 {
 	unsigned int metainfo_len;
 	struct _dbus_property *info;
@@ -384,7 +388,7 @@ LIB_EXPORT bool l_dbus_interface_property(struct l_dbus_interface *interface,
 	if (!_dbus_valid_method(name))
 		return false;
 
-	if (unlikely(!signature))
+	if (unlikely(!signature || !getter))
 		return false;
 
 	if (!_dbus_valid_signature(signature))
@@ -397,6 +401,8 @@ LIB_EXPORT bool l_dbus_interface_property(struct l_dbus_interface *interface,
 	info = l_malloc(sizeof(*info) + metainfo_len);
 	info->flags = flags;
 	info->name_len = strlen(name);
+	info->getter = getter;
+	info->setter = setter;
 
 	p = stpcpy(info->metainfo, name) + 1;
 	strcpy(p, signature);
@@ -406,22 +412,6 @@ LIB_EXPORT bool l_dbus_interface_property(struct l_dbus_interface *interface,
 	return true;
 }
 
-LIB_EXPORT bool l_dbus_interface_ro_property(struct l_dbus_interface *interface,
-						const char *name,
-						const char *signature)
-{
-	return l_dbus_interface_property(interface, name, 0, signature);
-}
-
-LIB_EXPORT bool l_dbus_interface_rw_property(struct l_dbus_interface *interface,
-						const char *name,
-						const char *signature)
-{
-	return l_dbus_interface_property(interface, name,
-					L_DBUS_PROPERTY_FLAG_WRITABLE,
-					signature);
-}
-
 struct l_dbus_interface *_dbus_interface_new(const char *name)
 {
 	struct l_dbus_interface *interface;
diff --git a/ell/dbus-service.h b/ell/dbus-service.h
index ce53082..d1825dd 100644
--- a/ell/dbus-service.h
+++ b/ell/dbus-service.h
@@ -46,13 +46,27 @@ enum l_dbus_signal_flag {
 
 enum l_dbus_property_flag {
 	L_DBUS_PROPERTY_FLAG_DEPRECATED = 1,
-	L_DBUS_PROPERTY_FLAG_WRITABLE	= 2,
 };
 
 typedef struct l_dbus_message *(*l_dbus_interface_method_cb_t) (struct l_dbus *,
 						struct l_dbus_message *message,
 						void *user_data);
 
+typedef void (*l_dbus_property_complete_cb_t) (struct l_dbus *,
+						struct l_dbus_message *,
+						struct l_dbus_message *error);
+
+typedef void (*l_dbus_property_set_cb_t) (struct l_dbus *,
+					struct l_dbus_message *message,
+					struct l_dbus_message_iter *new_value,
+					l_dbus_property_complete_cb_t complete,
+					void *user_data);
+
+typedef bool (*l_dbus_property_get_cb_t) (struct l_dbus *,
+					struct l_dbus_message *message,
+					struct l_dbus_message_builder *builder,
+					void *user_data);
+
 bool l_dbus_interface_method(struct l_dbus_interface *interface,
 				const char *name, uint32_t flags,
 				l_dbus_interface_method_cb_t cb,
@@ -65,13 +79,9 @@ bool l_dbus_interface_signal(struct l_dbus_interface *interface,
 
 bool l_dbus_interface_property(struct l_dbus_interface *interface,
 				const char *name, uint32_t flags,
-				const char *signature);
-bool l_dbus_interface_ro_property(struct l_dbus_interface *interface,
-					const char *name,
-					const char *signature);
-bool l_dbus_interface_rw_property(struct l_dbus_interface *interface,
-					const char *name,
-					const char *signature);
+				const char *signature,
+				l_dbus_property_get_cb_t getter,
+				l_dbus_property_set_cb_t setter);
 
 #ifdef __cplusplus
 }
diff --git a/examples/dbus-service.c b/examples/dbus-service.c
index 40c1646..2620a96 100644
--- a/examples/dbus-service.c
+++ b/examples/dbus-service.c
@@ -187,6 +187,75 @@ static struct l_dbus_message *test_method_call(struct l_dbus *dbus,
 	return reply;
 }
 
+static bool test_string_getter(struct l_dbus *dbus,
+				struct l_dbus_message *message,
+				struct l_dbus_message_builder *builder,
+				void *user_data)
+{
+	struct test_data *test = user_data;
+
+	l_dbus_message_builder_append_basic(builder, 's', test->string);
+
+	return true;
+}
+
+static void test_string_setter(struct l_dbus *dbus,
+				struct l_dbus_message *message,
+				struct l_dbus_message_iter *new_value,
+				l_dbus_property_complete_cb_t complete,
+				void *user_data)
+{
+	const char *strvalue;
+	struct test_data *test = user_data;
+
+	if (!l_dbus_message_iter_get_variant(new_value, "s", &strvalue)) {
+		complete(dbus, message, l_dbus_message_new_error(message,
+					"org.test.InvalidArguments",
+					"String value expected"));
+		return;
+	}
+
+	l_info("New String value: %s", strvalue);
+	l_free(test->string);
+	test->string = l_strdup(strvalue);
+
+	complete(dbus, message, NULL);
+}
+
+static bool test_int_getter(struct l_dbus *dbus,
+				struct l_dbus_message *message,
+				struct l_dbus_message_builder *builder,
+				void *user_data)
+{
+	struct test_data *test = user_data;
+
+	l_dbus_message_builder_append_basic(builder, 'u', &test->integer);
+
+	return true;
+}
+
+static void test_int_setter(struct l_dbus *dbus,
+				struct l_dbus_message *message,
+				struct l_dbus_message_iter *new_value,
+				l_dbus_property_complete_cb_t complete,
+				void *user_data)
+{
+	uint32_t u;
+	struct test_data *test = user_data;
+
+	if (!l_dbus_message_iter_get_variant(new_value, "u", &u)) {
+		complete(dbus, message, l_dbus_message_new_error(message,
+					"org.test.InvalidArguments",
+					"Integer value expected"));
+		return;
+	}
+
+	l_info("New Integer value: %u", u);
+	test->integer = u;
+
+	complete(dbus, message, NULL);
+}
+
 static void setup_test_interface(struct l_dbus_interface *interface)
 {
 	l_dbus_interface_method(interface, "GetProperties", 0,
@@ -201,8 +270,10 @@ static void setup_test_interface(struct l_dbus_interface *interface)
 	l_dbus_interface_signal(interface, "PropertyChanged", 0,
 				"sv", "name", "value");
 
-	l_dbus_interface_rw_property(interface, "String", "s");
-	l_dbus_interface_rw_property(interface, "Integer", "u");
+	l_dbus_interface_property(interface, "String", 0, "s",
+					test_string_getter, test_string_setter);
+	l_dbus_interface_property(interface, "Integer", 0, "u",
+					test_int_getter, test_int_setter);
 }
 
 int main(int argc, char *argv[])
diff --git a/unit/test-dbus-service.c b/unit/test-dbus-service.c
index 7452269..ea91b87 100644
--- a/unit/test-dbus-service.c
+++ b/unit/test-dbus-service.c
@@ -315,6 +315,22 @@ static void test_dbus_object_tree_dispatch(const void *test_data)
 	_dbus_object_tree_free(tree);
 }
 
+static bool test_property_getter(struct l_dbus *dbus,
+					struct l_dbus_message *message,
+					struct l_dbus_message_builder *builder,
+					void *if_user_data)
+{
+	return true;
+}
+
+static void test_property_setter(struct l_dbus *dbus,
+					struct l_dbus_message *message,
+					struct l_dbus_message_iter *new_value,
+					l_dbus_property_complete_cb_t complete,
+					void *user_data)
+{
+}
+
 int main(int argc, char *argv[])
 {
 	int ret;
@@ -333,7 +349,9 @@ int main(int argc, char *argv[])
 
 	l_dbus_interface_signal(interface, "Changed", 0, "b", "new_value");
 
-	l_dbus_interface_rw_property(interface, "Bar", "y");
+	l_dbus_interface_property(interface, "Bar", 0, "y",
+					test_property_getter,
+					test_property_setter);
 
 	l_test_add("Test Frobate Introspection", test_introspect_method,
 			&frobate_test);
-- 
2.5.0

[PATCH 3/8] dbus: Separate interface registration from instantiation

[Andrew Zaborowski]

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

Currently l_dbus_register_interface performs three related actions:
 * if needed, creates a new object node in the dbus object tree
 * if needed, sets up internal structs for the new interface
 * adds the interface to the object

With this patch these are three spearate calls, although the first
is still performed automatically by l_dbus_add_interface if
l_dbus_register_object wasn't called first.  This is in preparation for
ObjectManager support.  With this the setup_func parameter and new
interface parameters don't need to be passed every time an interface is
instiated, only when it's being registered/created.

Note that while the client doesn't need to call l_dbus_register_object,
they still need to call l_dbus_unregister_object to free the object
because it's not freed automatically when the last interface gets
removed.  But they can skip the l_dbus_remove_interface calls
because the interfaces will be removed either way.
---
 ell/dbus-private.h       |  22 +++++-
 ell/dbus-service.c       | 185 +++++++++++++++++++++++++++++++++++------------
 ell/dbus.c               | 147 +++++++++++++++++++++++++++++++++++--
 ell/dbus.h               |  19 +++--
 examples/dbus-service.c  |  13 +++-
 unit/test-dbus-service.c |  15 ++--
 unit/test-kdbus.c        |   9 ++-
 7 files changed, 336 insertions(+), 74 deletions(-)

diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index e60d09e..3eb3847 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -169,11 +169,25 @@ struct object_node *_dbus_object_tree_lookup(struct _dbus_object_tree *tree,
 						const char *path);
 void _dbus_object_tree_prune_node(struct object_node *node);
 
-bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
-				const char *path, const char *interface,
+struct object_node *_dbus_object_tree_new_object(struct _dbus_object_tree *tree,
+						const char *path,
+						void *user_data,
+						void (*destroy) (void *));
+bool _dbus_object_tree_object_destroy(struct _dbus_object_tree *tree,
+						const char *path);
+
+bool _dbus_object_tree_register_interface(struct _dbus_object_tree *tree,
+				const char *interface,
 				void (*setup_func)(struct l_dbus_interface *),
-				void *user_data, void (*destroy) (void *));
-bool _dbus_object_tree_unregister(struct _dbus_object_tree *tree,
+				void (*destroy) (void *),
+				bool old_style_properties);
+bool _dbus_object_tree_unregister_interface(struct _dbus_object_tree *tree,
+						const char *interface);
+
+bool _dbus_object_tree_add_interface(struct _dbus_object_tree *tree,
+					const char *path, const char *interface,
+					void *user_data);
+bool _dbus_object_tree_remove_interface(struct _dbus_object_tree *tree,
 					const char *path,
 					const char *interface);
 
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 772e7a7..2ec2df9 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -72,6 +72,8 @@ struct l_dbus_interface {
 	struct l_queue *methods;
 	struct l_queue *signals;
 	struct l_queue *properties;
+	bool handle_old_style_properties;
+	void (*instance_destroy)(void *);
 	char name[];
 };
 
@@ -84,13 +86,14 @@ struct child_node {
 struct interface_instance {
 	struct l_dbus_interface *interface;
 	void *user_data;
-	void (*user_destroy) (void *);
 };
 
 struct object_node {
 	struct object_node *parent;
 	struct l_queue *instances;
 	struct child_node *children;
+	void *user_data;
+	void (*destroy) (void *);
 };
 
 struct _dbus_object_tree {
@@ -489,8 +492,8 @@ struct _dbus_property *_dbus_interface_find_property(struct l_dbus_interface *i,
 
 static void interface_instance_free(struct interface_instance *instance)
 {
-	if (instance->user_destroy)
-		instance->user_destroy(instance->user_data);
+	if (instance->interface->instance_destroy)
+		instance->interface->instance_destroy(instance->user_data);
 
 	l_free(instance);
 }
@@ -539,6 +542,9 @@ static void subtree_free(struct object_node *node)
 	l_queue_destroy(node->instances,
 			(l_queue_destroy_func_t) interface_instance_free);
 
+	if (node->destroy)
+		node->destroy(node->user_data);
+
 	l_free(node);
 }
 
@@ -658,69 +664,164 @@ void _dbus_object_tree_prune_node(struct object_node *node)
 	}
 }
 
-bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
-				const char *path, const char *interface,
+struct object_node *_dbus_object_tree_new_object(struct _dbus_object_tree *tree,
+						const char *path,
+						void *user_data,
+						void (*destroy) (void *))
+{
+	struct object_node *node;
+
+	if (!_dbus_valid_object_path(path))
+		return NULL;
+
+	if (l_hashmap_lookup(tree->objects, path))
+		return NULL;
+
+	node = _dbus_object_tree_makepath(tree, path);
+	node->user_data = user_data;
+	node->destroy = destroy;
+	node->instances = l_queue_new();
+
+	l_hashmap_insert(tree->objects, path, node);
+
+	return node;
+}
+
+bool _dbus_object_tree_object_destroy(struct _dbus_object_tree *tree,
+					const char *path)
+{
+	struct object_node *node;
+
+	node = l_hashmap_remove(tree->objects, path);
+	if (!node)
+		return false;
+
+	l_queue_destroy(node->instances,
+			(l_queue_destroy_func_t) interface_instance_free);
+	node->instances = NULL;
+
+	if (node->destroy) {
+		node->destroy(node->user_data);
+		node->destroy = NULL;
+	}
+
+	if (!node->children)
+		_dbus_object_tree_prune_node(node);
+
+	return true;
+}
+
+bool _dbus_object_tree_register_interface(struct _dbus_object_tree *tree,
+				const char *interface,
 				void (*setup_func)(struct l_dbus_interface *),
-				void *user_data, void (*destroy) (void *))
+				void (*destroy) (void *),
+				bool old_style_properties)
 {
-	struct object_node *object;
 	struct l_dbus_interface *dbi;
-	const struct l_queue_entry *entry;
-	struct interface_instance *instance;
 
 	if (!_dbus_valid_interface(interface))
 		return false;
 
-	if (!_dbus_valid_object_path(path))
+	/*
+	 * Check to make sure we do not have this interface already
+	 * registered
+	 */
+	dbi = l_hashmap_lookup(tree->interfaces, interface);
+	if (dbi)
+		return false;
+
+	dbi = _dbus_interface_new(interface);
+	dbi->instance_destroy = destroy;
+	dbi->handle_old_style_properties = old_style_properties;
+
+	setup_func(dbi);
+
+	l_hashmap_insert(tree->interfaces, dbi->name, dbi);
+
+	return true;
+}
+
+struct interface_check {
+	struct _dbus_object_tree *tree;
+	const char *interface;
+};
+
+static void check_interface_used(const void *key, void *value, void *user_data)
+{
+	const char *path = key;
+	struct object_node *node = value;
+	struct interface_check *state = user_data;
+
+	if (!l_queue_find(node->instances, match_interface_instance,
+				(char *) state->interface))
+		return;
+
+	_dbus_object_tree_remove_interface(state->tree, path, state->interface);
+}
+
+bool _dbus_object_tree_unregister_interface(struct _dbus_object_tree *tree,
+						const char *interface_name)
+{
+	struct l_dbus_interface *interface;
+	struct interface_check state = { tree, interface_name };
+
+	interface = l_hashmap_lookup(tree->interfaces, interface_name);
+	if (!interface)
+		return false;
+
+	/* Check that the interface is not in use */
+	l_hashmap_foreach(tree->objects, check_interface_used, &state);
+
+	l_hashmap_remove(tree->interfaces, interface_name);
+
+	_dbus_interface_free(interface);
+
+	return true;
+}
+
+bool _dbus_object_tree_add_interface(struct _dbus_object_tree *tree,
+					const char *path, const char *interface,
+					void *user_data)
+{
+	struct object_node *object;
+	struct l_dbus_interface *dbi;
+	struct interface_instance *instance;
+
+	dbi = l_hashmap_lookup(tree->interfaces, interface);
+	if (!dbi)
 		return false;
 
 	object = l_hashmap_lookup(tree->objects, path);
 	if (!object) {
-		object = _dbus_object_tree_makepath(tree, path);
-		l_hashmap_insert(tree->objects, path, object);
+		object = _dbus_object_tree_new_object(tree, path, NULL, NULL);
+
+		if (!object)
+			return false;
 	}
 
 	/*
 	 * Check to make sure we do not have this interface already
 	 * registered for this object
 	 */
-	entry = l_queue_get_entries(object->instances);
-	while (entry) {
-		instance = entry->data;
-
-		if (!strcmp(instance->interface->name, interface))
-			return false;
-
-		entry = entry->next;
-	}
-
-	dbi = l_hashmap_lookup(tree->interfaces, interface);
-	if (!dbi) {
-		dbi = _dbus_interface_new(interface);
-		setup_func(dbi);
-		l_hashmap_insert(tree->interfaces, dbi->name, dbi);
-	}
+	if (l_queue_find(object->instances, match_interface_instance,
+				(char *) interface))
+		return false;
 
 	instance = l_new(struct interface_instance, 1);
 	instance->interface = dbi;
-	instance->user_destroy = destroy;
 	instance->user_data = user_data;
 
-	if (!object->instances)
-		object->instances = l_queue_new();
-
 	l_queue_push_tail(object->instances, instance);
 
 	return true;
 }
 
-bool _dbus_object_tree_unregister(struct _dbus_object_tree *tree,
+bool _dbus_object_tree_remove_interface(struct _dbus_object_tree *tree,
 					const char *path,
 					const char *interface)
 {
 	struct object_node *node;
 	struct interface_instance *instance;
-	bool r;
 
 	node = l_hashmap_lookup(tree->objects, path);
 	if (!node)
@@ -728,20 +829,12 @@ bool _dbus_object_tree_unregister(struct _dbus_object_tree *tree,
 
 	instance = l_queue_remove_if(node->instances,
 			match_interface_instance, (char *) interface);
+	if (!instance)
+		return false;
 
-	r = instance ? true : false;
-
-	if (instance)
-		interface_instance_free(instance);
-
-	if (l_queue_isempty(node->instances)) {
-		l_hashmap_remove(tree->objects, path);
+	interface_instance_free(instance);
 
-		if (!node->children)
-			_dbus_object_tree_prune_node(node);
-	}
-
-	return r;
+	return true;
 }
 
 static void generate_interface_instance(void *data, void *user)
diff --git a/ell/dbus.c b/ell/dbus.c
index 1d9aed5..d825414 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -1214,11 +1214,28 @@ int _dbus_get_fd(struct l_dbus *dbus)
 	return l_io_get_fd(dbus->io);
 }
 
+/**
+ * l_dbus_register_interface:
+ * @dbus: D-Bus connection as returned by @l_dbus_new*
+ * @interface: interface name string
+ * @setup_func: function that sets up the methods, signals and properties by
+ *              using the #dbus-service.h API.
+ * @destroy: optional destructor to be called every time an instance of this
+ *           interface is being removed from an object on this bus.
+ * @handle_old_style_properties: whether to automatically handle SetProperty and
+ *                               GetProperties for any properties registered by
+ *                               @setup_func.
+ *
+ * Registers an interface.  If successful the interface can then be added
+ * to any number of objects with @l_dbus_object_add_interface.
+ *
+ * Returns: whether the interface was successfully registered
+ **/
 LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
-				const char *path, const char *interface,
+				const char *interface,
 				l_dbus_interface_setup_func_t setup_func,
-				void *user_data,
-				l_dbus_destroy_func_t destroy)
+				l_dbus_destroy_func_t destroy,
+				bool handle_old_style_properties)
 {
 	if (unlikely(!dbus))
 		return false;
@@ -1226,12 +1243,12 @@ LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
 	if (unlikely(!dbus->tree))
 		return false;
 
-	return _dbus_object_tree_register(dbus->tree, path, interface,
-						setup_func, user_data, destroy);
+	return _dbus_object_tree_register_interface(dbus->tree, interface,
+						setup_func, destroy,
+						handle_old_style_properties);
 }
 
 LIB_EXPORT bool l_dbus_unregister_interface(struct l_dbus *dbus,
-						const char *path,
 						const char *interface)
 {
 	if (unlikely(!dbus))
@@ -1240,7 +1257,123 @@ LIB_EXPORT bool l_dbus_unregister_interface(struct l_dbus *dbus,
 	if (unlikely(!dbus->tree))
 		return false;
 
-	return _dbus_object_tree_unregister(dbus->tree, path, interface);
+	return _dbus_object_tree_unregister_interface(dbus->tree, interface);
+}
+
+/**
+ * l_dbus_register_object:
+ * @dbus: D-Bus connection
+ * @path: new object path
+ * @user_data: user pointer to be passed to @destroy if any
+ * @destroy: optional destructor to be called when object dropped from the tree
+ * @...: NULL-terminated list of 0 or more interfaces to be present on the
+ *       object from the moment of creation.  For every interface the interface
+ *       name string is expected followed by the @user_data pointer same as
+ *       would be passed as @l_dbus_object_add_interface's last two parameters.
+ *
+ * Create a new D-Bus object on the tree visible to D-Bus peers.  For example:
+ * 	success = l_dbus_register_object(bus, "/org/example/ExampleManager",
+ * 						NULL, NULL,
+ * 						"org.example.Manager",
+ * 						manager_data,
+ * 						NULL);
+ *
+ * Returns: whether the object path was successfully registered
+ **/
+LIB_EXPORT bool l_dbus_register_object(struct l_dbus *dbus, const char *path,
+					void *user_data,
+					l_dbus_destroy_func_t destroy, ...)
+{
+	va_list args;
+	const char *interface;
+	void *if_user_data;
+	bool r = true;;
+
+	if (unlikely(!dbus))
+		return false;
+
+	if (unlikely(!dbus->tree))
+		return false;
+
+	if (!_dbus_object_tree_new_object(dbus->tree, path, user_data, destroy))
+		return false;
+
+	va_start(args, destroy);
+	while ((interface = va_arg(args, const char *))) {
+		if_user_data = va_arg(args, void *);
+
+		if (!_dbus_object_tree_add_interface(dbus->tree, path,
+							interface,
+							if_user_data)) {
+			_dbus_object_tree_object_destroy(dbus->tree, path);
+			r = false;
+
+			break;
+		}
+	}
+	va_end(args);
+
+	return r;
+}
+
+LIB_EXPORT bool l_dbus_unregister_object(struct l_dbus *dbus,
+						const char *object)
+{
+	if (unlikely(!dbus))
+		return false;
+
+	if (unlikely(!dbus->tree))
+		return false;
+
+	return _dbus_object_tree_object_destroy(dbus->tree, object);
+}
+
+/**
+ * l_dbus_object_add_interface:
+ * @dbus: D-Bus connection
+ * @object: object path as passed to @l_dbus_register_object
+ * @interface: interface name as passed to @l_dbus_register_interface
+ * @user_data: user data pointer to be passed to any method and property
+ *             callbacks provided by the @setup_func and to the @destroy
+ *             callback as passed to @l_dbus_register_interface
+ *
+ * Creates an instance of given interface at the given path in the
+ * connection's object tree.  If no object was registered at this path
+ * before @l_dbus_register_object gets called automatically.
+ *
+ * The addition of an interface to the object may trigger a query of
+ * all the properties on this interface and
+ * #org.freedesktop.DBus.ObjectManager.InterfacesAdded signals.
+ *
+ * Returns: whether the interface was successfully added.
+ **/
+LIB_EXPORT bool l_dbus_object_add_interface(struct l_dbus *dbus,
+						const char *object,
+						const char *interface,
+						void *user_data)
+{
+	if (unlikely(!dbus))
+		return false;
+
+	if (unlikely(!dbus->tree))
+		return false;
+
+	return _dbus_object_tree_add_interface(dbus->tree, object, interface,
+						user_data);
+}
+
+LIB_EXPORT bool l_dbus_object_remove_interface(struct l_dbus *dbus,
+						const char *object,
+						const char *interface)
+{
+	if (unlikely(!dbus))
+		return false;
+
+	if (unlikely(!dbus->tree))
+		return false;
+
+	return _dbus_object_tree_remove_interface(dbus->tree, object,
+							interface);
 }
 
 void _dbus1_filter_format_match(struct dbus1_filter_data *data, char *rule,
diff --git a/ell/dbus.h b/ell/dbus.h
index 39f926b..011d59a 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -191,13 +191,22 @@ bool l_dbus_message_builder_leave_variant(
 struct l_dbus_message *l_dbus_message_builder_finalize(
 					struct l_dbus_message_builder *builder);
 
-bool l_dbus_register_interface(struct l_dbus *dbus,
-				const char *path, const char *interface,
+bool l_dbus_register_interface(struct l_dbus *dbus, const char *interface,
 				l_dbus_interface_setup_func_t setup_func,
-				void *user_data,
-				l_dbus_destroy_func_t destroy);
-bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *path,
+				l_dbus_destroy_func_t destroy,
+				bool handle_old_style_properties);
+bool l_dbus_unregister_interface(struct l_dbus *dbus, const char *interface);
+
+bool l_dbus_register_object(struct l_dbus *dbus, const char *path,
+				void *user_data, l_dbus_destroy_func_t destroy,
+				...);
+bool l_dbus_unregister_object(struct l_dbus *dbus, const char *object);
+
+bool l_dbus_object_add_interface(struct l_dbus *dbus, const char *object,
+					const char *interface, void *user_data);
+bool l_dbus_object_remove_interface(struct l_dbus *dbus, const char *object,
 					const char *interface);
+
 unsigned int l_dbus_add_disconnect_watch(struct l_dbus *dbus,
 					const char *name,
 					l_dbus_watch_func_t disconnect_func,
diff --git a/examples/dbus-service.c b/examples/dbus-service.c
index 2620a96..5b8b086 100644
--- a/examples/dbus-service.c
+++ b/examples/dbus-service.c
@@ -305,17 +305,22 @@ int main(int argc, char *argv[])
 	test->string = l_strdup("Default");
 	test->integer = 42;
 
-	if (!l_dbus_register_interface(dbus, "/test", "org.test",
-					setup_test_interface, test,
-					test_data_destroy)) {
+	if (!l_dbus_register_interface(dbus, "org.test", setup_test_interface,
+					test_data_destroy, false)) {
 		l_info("Unable to register interface");
 		test_data_destroy(test);
 		goto cleanup;
 	}
 
+	if (!l_dbus_object_add_interface(dbus, "/test", "org.test", test)) {
+		l_info("Unable to instantiate interface");
+		test_data_destroy(test);
+		goto cleanup;
+	}
+
 	l_main_run();
 
-	l_dbus_unregister_interface(dbus, "/test", "org.test");
+	l_dbus_unregister_object(dbus, "/test");
 
 cleanup:
 	l_dbus_destroy(dbus);
diff --git a/unit/test-dbus-service.c b/unit/test-dbus-service.c
index ea91b87..2438e68 100644
--- a/unit/test-dbus-service.c
+++ b/unit/test-dbus-service.c
@@ -276,9 +276,10 @@ static void test_dbus_object_tree_introspection(const void *test_data)
 
 	tree = _dbus_object_tree_new();
 
-	_dbus_object_tree_register(tree, "/", "org.ofono.Manager",
-					build_manager_interface,
-					NULL, NULL);
+	_dbus_object_tree_register_interface(tree, "org.ofono.Manager",
+						build_manager_interface,
+						NULL, false);
+	_dbus_object_tree_add_interface(tree, "/", "org.ofono.Manager", NULL);
 
 	_dbus_object_tree_makepath(tree, "/phonesim");
 
@@ -298,9 +299,11 @@ static void test_dbus_object_tree_dispatch(const void *test_data)
 
 	tree = _dbus_object_tree_new();
 
-	_dbus_object_tree_register(tree, "/", "org.ofono.Manager",
-					build_manager_interface,
-					dummy_data, NULL);
+	_dbus_object_tree_register_interface(tree, "org.ofono.Manager",
+						build_manager_interface,
+						NULL, false);
+	_dbus_object_tree_add_interface(tree, "/", "org.ofono.Manager",
+					dummy_data);
 
 	message = _dbus_message_new_method_call(1, "org.ofono", "/",
 						"org.ofono.Manager",
diff --git a/unit/test-kdbus.c b/unit/test-kdbus.c
index 4bf41bb..3a6af3e 100644
--- a/unit/test-kdbus.c
+++ b/unit/test-kdbus.c
@@ -158,12 +158,17 @@ int main(int argc, char *argv[])
 	l_dbus_set_ready_handler(service, service_ready_callback,
 					service, NULL);
 
-	if (!l_dbus_register_interface(service, "/test", "org.test",
-					setup_test_interface, NULL, NULL)) {
+	if (!l_dbus_register_interface(service, "org.test",
+					setup_test_interface, NULL, false)) {
 		l_info("Unable to register interface");
 		goto error;
 	}
 
+	if (!l_dbus_object_add_interface(service, "/test", "org.test", NULL)) {
+		l_info("Unable to instantiate interface");
+		goto error;
+	}
+
 	client = l_dbus_new(bus_address);
 	assert(client);
 
-- 
2.5.0

[PATCH 4/8] dbus: Message builder function to copy from an iter.

[Andrew Zaborowski]

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

---
 ell/dbus-message.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 ell/dbus.h         |   4 ++
 2 files changed, 129 insertions(+)

diff --git a/ell/dbus-message.c b/ell/dbus-message.c
index 159df82..e618cf7 100644
--- a/ell/dbus-message.c
+++ b/ell/dbus-message.c
@@ -1584,6 +1584,131 @@ LIB_EXPORT bool l_dbus_message_builder_leave_variant(
 	return l_dbus_message_builder_leave_container(builder, 'v');
 }
 
+/**
+ * l_dbus_message_builder_append_from_iter:
+ * @builder: message builder to receive a new value
+ * @from: message iterator to have its position moved by one value
+ *
+ * Copy one value from a message iterator onto a message builder.  The
+ * value's signature is also copied.
+ *
+ * Returns: whether the value was correctly copied.  On failure both
+ *          the @from iterator and the @builder may have their positions
+ *          moved to somewhere within the new value if it's of a
+ *          container type.
+ **/
+LIB_EXPORT bool l_dbus_message_builder_append_from_iter(
+					struct l_dbus_message_builder *builder,
+					struct l_dbus_message_iter *from)
+{
+	static const char *simple_types = "sogybnqiuxtd";
+	char type = from->sig_start[from->sig_pos];
+	char container_type;
+	char signature[256];
+	struct l_dbus_message_iter iter;
+	void *basic_ptr;
+	uint64_t basic;
+	uint32_t uint32_val;
+	int fd;
+	bool (*get_basic)(struct l_dbus_message_iter *, char, void *);
+	bool (*enter_func)(struct l_dbus_message_iter *,
+				struct l_dbus_message_iter *);
+	bool (*enter_struct)(struct l_dbus_message_iter *,
+				struct l_dbus_message_iter *);
+	bool (*enter_array)(struct l_dbus_message_iter *,
+				struct l_dbus_message_iter *);
+	bool (*enter_variant)(struct l_dbus_message_iter *,
+				struct l_dbus_message_iter *);
+
+	if (_dbus_message_is_gvariant(from->message)) {
+		get_basic = _gvariant_iter_next_entry_basic;
+		enter_struct = _gvariant_iter_enter_struct;
+		enter_array = _gvariant_iter_enter_array;
+		enter_variant = _gvariant_iter_enter_variant;
+	} else {
+		get_basic = _dbus1_iter_next_entry_basic;
+		enter_struct = _dbus1_iter_enter_struct;
+		enter_array = _dbus1_iter_enter_array;
+		enter_variant = _dbus1_iter_enter_variant;
+	}
+
+	if (strchr(simple_types, type)) {
+		if (strchr("sog", type)) {
+			if (!get_basic(from, type, &basic_ptr))
+				return false;
+		} else {
+			basic_ptr = &basic;
+
+			if (!get_basic(from, type, basic_ptr))
+				return false;
+		}
+
+		if (!l_dbus_message_builder_append_basic(builder, type,
+								basic_ptr))
+			return false;
+
+		return true;
+	}
+
+	switch (type) {
+	case 'h':
+		if (!get_basic(from, type, &uint32_val))
+			return false;
+
+		if (uint32_val >= from->message->num_fds)
+			return false;
+
+		fd = fcntl(from->message->fds[uint32_val], F_DUPFD_CLOEXEC, 3);
+
+		uint32_val = builder->message->num_fds;
+		builder->message->fds[builder->message->num_fds++] = fd;
+
+		if (!l_dbus_message_builder_append_basic(builder, type,
+								&uint32_val))
+			return false;
+
+		return true;
+	case '(':
+		enter_func = enter_struct;
+		container_type = DBUS_CONTAINER_TYPE_STRUCT;
+		break;
+	case '{':
+		enter_func = enter_struct;
+		container_type = DBUS_CONTAINER_TYPE_DICT_ENTRY;
+		break;
+	case 'a':
+		enter_func = enter_array;
+		container_type = DBUS_CONTAINER_TYPE_ARRAY;
+		break;
+	case 'v':
+		enter_func = enter_variant;
+		container_type = DBUS_CONTAINER_TYPE_VARIANT;
+		break;
+	default:
+		return false;
+	}
+
+	if (!enter_func(from, &iter))
+		return false;
+
+	memcpy(signature, iter.sig_start, iter.sig_len);
+	signature[iter.sig_len] = '\0';
+
+	if (!l_dbus_message_builder_enter_container(builder,
+						container_type, signature))
+		return false;
+
+	while (iter.sig_pos < iter.sig_len)
+		if (!l_dbus_message_builder_append_from_iter(builder, &iter))
+			return false;
+
+	if (!l_dbus_message_builder_leave_container(builder,
+						container_type))
+		return false;
+
+	return true;
+}
+
 LIB_EXPORT struct l_dbus_message *l_dbus_message_builder_finalize(
 					struct l_dbus_message_builder *builder)
 {
diff --git a/ell/dbus.h b/ell/dbus.h
index 011d59a..29253b6 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -188,6 +188,10 @@ bool l_dbus_message_builder_enter_variant(
 bool l_dbus_message_builder_leave_variant(
 					struct l_dbus_message_builder *builder);
 
+bool l_dbus_message_builder_append_from_iter(
+					struct l_dbus_message_builder *builder,
+					struct l_dbus_message_iter *from);
+
 struct l_dbus_message *l_dbus_message_builder_finalize(
 					struct l_dbus_message_builder *builder);
 
-- 
2.5.0

[PATCH 5/8] dbus: Private function to retrieve the tree for a connection.

[Andrew Zaborowski]

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

---
 ell/dbus-private.h | 1 +
 ell/dbus.c         | 5 +++++
 2 files changed, 6 insertions(+)

diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 3eb3847..8c58218 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -226,6 +226,7 @@ int _dbus_kernel_remove_match(int fd, uint64_t cookie);
 
 uint8_t _dbus_get_version(struct l_dbus *dbus);
 int _dbus_get_fd(struct l_dbus *dbus);
+struct _dbus_object_tree *_dbus_get_tree(struct l_dbus *dbus);
 
 struct dbus1_filter_data;
 
diff --git a/ell/dbus.c b/ell/dbus.c
index d825414..796240f 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -1214,6 +1214,11 @@ int _dbus_get_fd(struct l_dbus *dbus)
 	return l_io_get_fd(dbus->io);
 }
 
+struct _dbus_object_tree *_dbus_get_tree(struct l_dbus *dbus)
+{
+	return dbus->tree;
+}
+
 /**
  * l_dbus_register_interface:
  * @dbus: D-Bus connection as returned by @l_dbus_new*
-- 
2.5.0

[PATCH 6/8] dbus: Handle legacy GetProperties and SetProperty automatically.

[Andrew Zaborowski]

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

---
 ell/dbus-private.h      |   6 ++
 ell/dbus-service.c      | 190 ++++++++++++++++++++++++++++++++++++++++++++++++
 examples/dbus-service.c |  86 +---------------------
 3 files changed, 197 insertions(+), 85 deletions(-)

diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 8c58218..1af7201 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -197,6 +197,12 @@ bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
 					struct l_dbus *dbus,
 					struct l_dbus_message *message);
 
+void _dbus_object_tree_property_changed(struct l_dbus *dbus,
+					const char *path,
+					const char *interface_name,
+					const char *property_name,
+					struct l_dbus_message_iter *variant);
+
 void _dbus_kernel_bloom_add(uint64_t filter[], size_t size, uint8_t num_hash,
 				const char *prefix, const char *str);
 void _dbus_kernel_bloom_add_parents(uint64_t filter[], size_t size,
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 2ec2df9..c9e465c 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -711,6 +711,183 @@ bool _dbus_object_tree_object_destroy(struct _dbus_object_tree *tree,
 	return true;
 }
 
+void _dbus_object_tree_property_changed(struct l_dbus *dbus,
+					const char *path,
+					const char *interface_name,
+					const char *property_name,
+					struct l_dbus_message_iter *variant)
+{
+	struct l_dbus_message *signal;
+	struct l_dbus_message_builder *builder;
+	char signature[256];
+	const struct l_dbus_interface *interface;
+	struct l_dbus_message_iter value;
+	struct _dbus_object_tree *tree = _dbus_get_tree(dbus);
+
+	memcpy(signature, variant->sig_start, variant->sig_len);
+	signature[variant->sig_len] = '\0';
+
+	interface = l_hashmap_lookup(tree->interfaces, interface_name);
+
+	if (interface->handle_old_style_properties) {
+		memcpy(&value, variant, sizeof(value));
+
+		signal = l_dbus_message_new_signal(dbus, path,
+							interface_name,
+							"PropertyChanged");
+
+		builder = l_dbus_message_builder_new(signal);
+
+		l_dbus_message_builder_append_basic(builder, 's',
+							property_name);
+		l_dbus_message_builder_enter_variant(builder, signature);
+		l_dbus_message_builder_append_from_iter(builder, &value);
+		l_dbus_message_builder_leave_variant(builder);
+
+		l_dbus_message_builder_finalize(builder);
+		l_dbus_message_builder_destroy(builder);
+		l_dbus_send(dbus, signal);
+	}
+}
+
+static void set_property_complete(struct l_dbus *dbus,
+					struct l_dbus_message *message,
+					struct l_dbus_message *error)
+{
+	struct l_dbus_message *reply;
+	const char *property_name;
+	struct l_dbus_message_iter variant;
+
+	if (error) {
+		l_dbus_message_unref(message);
+
+		l_dbus_send(dbus, error);
+
+		return;
+	}
+
+	reply = l_dbus_message_new_method_return(message);
+	l_dbus_message_set_arguments(reply, "");
+	l_dbus_send(dbus, reply);
+
+	l_dbus_message_get_arguments(message, "sv", &property_name, &variant);
+
+	_dbus_object_tree_property_changed(dbus,
+					l_dbus_message_get_path(message),
+					l_dbus_message_get_interface(message),
+					property_name, &variant);
+
+	l_dbus_message_unref(message);
+}
+
+static struct l_dbus_message *old_set_property(struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	struct l_dbus_interface *interface;
+	const char *property_name;
+	const struct _dbus_property *property;
+	struct l_dbus_message_iter variant;
+	struct _dbus_object_tree *tree = _dbus_get_tree(dbus);
+
+	interface = l_hashmap_lookup(tree->interfaces,
+					l_dbus_message_get_interface(message));
+	/* If we got here the interface must exist */
+
+	if (!l_dbus_message_get_arguments(message, "sv", &property_name,
+						&variant))
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Invalid arguments");
+
+	property = _dbus_interface_find_property(interface, property_name);
+	if (!property)
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Unknown Property %s",
+						property_name);
+
+	if (!property->setter)
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Property %s is read-only",
+						property_name);
+
+	property->setter(dbus, l_dbus_message_ref(message), &variant,
+				set_property_complete, user_data);
+
+	return NULL;
+}
+
+static bool get_properties_dict(struct l_dbus *dbus,
+				struct l_dbus_message *message,
+				struct l_dbus_message_builder *builder,
+				const struct l_dbus_interface *interface,
+				void *user_data)
+{
+	const struct l_queue_entry *entry;
+	const struct _dbus_property *property;
+	const char *signature;
+	bool r = true;
+
+	l_dbus_message_builder_enter_array(builder, "{sv}");
+
+	for (entry = l_queue_get_entries(interface->properties); entry && r;
+			entry = entry->next) {
+		property = entry->data;
+		signature = property->metainfo + strlen(property->metainfo) + 1;
+
+		l_dbus_message_builder_enter_dict(builder, "sv");
+		l_dbus_message_builder_append_basic(builder, 's',
+							property->metainfo);
+		l_dbus_message_builder_enter_variant(builder, signature);
+
+		r = property->getter(dbus, message, builder, user_data);
+
+		l_dbus_message_builder_leave_variant(builder);
+		l_dbus_message_builder_leave_dict(builder);
+	}
+
+	l_dbus_message_builder_leave_array(builder);
+
+	return true;
+}
+
+static struct l_dbus_message *old_get_properties(struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	const struct l_dbus_interface *interface;
+	struct l_dbus_message *reply;
+	struct l_dbus_message_builder *builder;
+	struct _dbus_object_tree *tree = _dbus_get_tree(dbus);
+
+	interface = l_hashmap_lookup(tree->interfaces,
+					l_dbus_message_get_interface(message));
+	/* If we got here the interface must exist */
+
+	reply = l_dbus_message_new_method_return(message);
+	builder = l_dbus_message_builder_new(reply);
+
+	if (!get_properties_dict(dbus, message, builder, interface,
+					user_data)) {
+		l_dbus_message_unref(reply);
+
+		reply = l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"Failed",
+						"Getting properties failed");
+	}
+
+	l_dbus_message_builder_finalize(builder);
+	l_dbus_message_builder_destroy(builder);
+
+	return reply;
+}
+
 bool _dbus_object_tree_register_interface(struct _dbus_object_tree *tree,
 				const char *interface,
 				void (*setup_func)(struct l_dbus_interface *),
@@ -734,6 +911,19 @@ bool _dbus_object_tree_register_interface(struct _dbus_object_tree *tree,
 	dbi->instance_destroy = destroy;
 	dbi->handle_old_style_properties = old_style_properties;
 
+	/* Add our methods first so we don't have to check for conflicts. */
+	if (old_style_properties) {
+		l_dbus_interface_method(dbi, "SetProperty", 0,
+					old_set_property, "", "sv",
+					"name", "value");
+		l_dbus_interface_method(dbi, "GetProperties", 0,
+					old_get_properties, "a{sv}", "",
+					"properties");
+
+		l_dbus_interface_signal(dbi, "PropertyChanged", 0, "sv",
+					"name", "value");
+	}
+
 	setup_func(dbi);
 
 	l_hashmap_insert(tree->interfaces, dbi->name, dbi);
diff --git a/examples/dbus-service.c b/examples/dbus-service.c
index 5b8b086..8abeba4 100644
--- a/examples/dbus-service.c
+++ b/examples/dbus-service.c
@@ -98,81 +98,6 @@ static void test_data_destroy(void *data)
 	l_free(test);
 }
 
-static struct l_dbus_message *test_set_property(struct l_dbus *dbus,
-						struct l_dbus_message *message,
-						void *user_data)
-{
-	struct test_data *test = user_data;
-	struct l_dbus_message *reply;
-	struct l_dbus_message *signal;
-	struct l_dbus_message_iter variant;
-	const char *property;
-
-	if (!l_dbus_message_get_arguments(message, "sv", &property, &variant))
-		return l_dbus_message_new_error(message,
-						"org.test.InvalidArguments",
-						"Invalid arguments");
-
-	if (!strcmp(property, "String")) {
-		const char *strvalue;
-
-		if (!l_dbus_message_iter_get_variant(&variant, "s",
-							&strvalue))
-			return l_dbus_message_new_error(message,
-						"org.test.InvalidArguments",
-						"String value expected");
-
-		l_info("New String value: %s", strvalue);
-		l_free(test->string);
-		test->string = l_strdup(strvalue);
-
-		signal = l_dbus_message_new_signal(dbus, "/test",
-					"org.test", "PropertyChanged");
-		l_dbus_message_set_arguments(signal, "sv",
-						"String", "s", test->string);
-	} else if (!strcmp(property, "Integer")) {
-		uint32_t u;
-
-		if (!l_dbus_message_iter_get_variant(&variant, "u", &u))
-			return l_dbus_message_new_error(message,
-						"org.test.InvalidArguments",
-						"Integer value expected");
-
-		l_info("New Integer value: %u", u);
-		test->integer = u;
-		signal = l_dbus_message_new_signal(dbus, "/test",
-					"org.test", "PropertyChanged");
-		l_dbus_message_set_arguments(signal, "sv",
-						"Integer", "u", test->integer);
-	} else
-		return l_dbus_message_new_error(message,
-						"org.test.InvalidArguments",
-						"Unknown Property %s",
-						property);
-
-	reply = l_dbus_message_new_method_return(message);
-	l_dbus_message_set_arguments(reply, "");
-	l_dbus_send(dbus, reply);
-
-	l_dbus_send(dbus, signal);
-	return NULL;
-}
-
-static struct l_dbus_message *test_get_properties(struct l_dbus *dbus,
-						struct l_dbus_message *message,
-						void *user_data)
-{
-	struct test_data *test = user_data;
-	struct l_dbus_message *reply;
-
-	reply = l_dbus_message_new_method_return(message);
-	l_dbus_message_set_arguments(reply, "a{sv}", 2,
-					"String", "s", test->string,
-					"Integer", "u", test->integer);
-
-	return reply;
-}
-
 static struct l_dbus_message *test_method_call(struct l_dbus *dbus,
 						struct l_dbus_message *message,
 						void *user_data)
@@ -258,18 +183,9 @@ static void test_int_setter(struct l_dbus *dbus,
 
 static void setup_test_interface(struct l_dbus_interface *interface)
 {
-	l_dbus_interface_method(interface, "GetProperties", 0,
-				test_get_properties,
-				"a{sv}", "", "properties");
-	l_dbus_interface_method(interface, "SetProperty", 0,
-				test_set_property,
-				"", "sv", "name", "value");
 	l_dbus_interface_method(interface, "MethodCall", 0,
 				test_method_call, "", "");
 
-	l_dbus_interface_signal(interface, "PropertyChanged", 0,
-				"sv", "name", "value");
-
 	l_dbus_interface_property(interface, "String", 0, "s",
 					test_string_getter, test_string_setter);
 	l_dbus_interface_property(interface, "Integer", 0, "u",
@@ -306,7 +222,7 @@ int main(int argc, char *argv[])
 	test->integer = 42;
 
 	if (!l_dbus_register_interface(dbus, "org.test", setup_test_interface,
-					test_data_destroy, false)) {
+					test_data_destroy, true)) {
 		l_info("Unable to register interface");
 		test_data_destroy(test);
 		goto cleanup;
-- 
2.5.0

[PATCH 7/8] dbus: Implement org.freedesktop.DBus.Properties

[Andrew Zaborowski]

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

---
 ell/dbus-service.c      | 270 ++++++++++++++++++++++++++++++++++++++++++++++++
 ell/dbus.c              |   1 -
 examples/dbus-service.c |   7 ++
 3 files changed, 277 insertions(+), 1 deletion(-)

diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index c9e465c..2b38e0a 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -47,6 +47,8 @@ static const char *static_introspectable =
 		"\t\t\t<arg name=\"xml\" type=\"s\" direction=\"out\"/>\n"
 		"\t\t</method>\n\t</interface>\n";
 
+#define DBUS_INTERFACE_PROPERTIES	"org.freedesktop.DBus.Properties"
+
 struct _dbus_method {
 	l_dbus_interface_method_cb_t cb;
 	uint32_t flags;
@@ -509,6 +511,8 @@ static bool match_interface_instance(const void *a, const void *b)
 	return false;
 }
 
+static void properties_setup_func(struct l_dbus_interface *);
+
 struct _dbus_object_tree *_dbus_object_tree_new()
 {
 	struct _dbus_object_tree *tree;
@@ -524,6 +528,10 @@ struct _dbus_object_tree *_dbus_object_tree_new()
 
 	tree->root = l_new(struct object_node, 1);
 
+	_dbus_object_tree_register_interface(tree, DBUS_INTERFACE_PROPERTIES,
+						properties_setup_func, NULL,
+						false);
+
 	return tree;
 }
 
@@ -711,6 +719,7 @@ bool _dbus_object_tree_object_destroy(struct _dbus_object_tree *tree,
 	return true;
 }
 
+/* Send the signals associated with a property value change */
 void _dbus_object_tree_property_changed(struct l_dbus *dbus,
 					const char *path,
 					const char *interface_name,
@@ -721,6 +730,7 @@ void _dbus_object_tree_property_changed(struct l_dbus *dbus,
 	struct l_dbus_message_builder *builder;
 	char signature[256];
 	const struct l_dbus_interface *interface;
+	const struct object_node *object;
 	struct l_dbus_message_iter value;
 	struct _dbus_object_tree *tree = _dbus_get_tree(dbus);
 
@@ -748,6 +758,37 @@ void _dbus_object_tree_property_changed(struct l_dbus *dbus,
 		l_dbus_message_builder_destroy(builder);
 		l_dbus_send(dbus, signal);
 	}
+
+	object = l_hashmap_lookup(tree->objects, path);
+
+	if (l_queue_find(object->instances, match_interface_instance,
+				DBUS_INTERFACE_PROPERTIES)) {
+		memcpy(&value, variant, sizeof(value));
+
+		signal = l_dbus_message_new_signal(dbus, path,
+						DBUS_INTERFACE_PROPERTIES,
+						"PropertiesChanged");
+
+		builder = l_dbus_message_builder_new(signal);
+
+		l_dbus_message_builder_append_basic(builder, 's',
+							interface_name);
+		l_dbus_message_builder_enter_array(builder, "{sv}");
+		l_dbus_message_builder_enter_dict(builder, "sv");
+		l_dbus_message_builder_append_basic(builder, 's',
+							property_name);
+		l_dbus_message_builder_enter_variant(builder, signature);
+		l_dbus_message_builder_append_from_iter(builder, &value);
+		l_dbus_message_builder_leave_variant(builder);
+		l_dbus_message_builder_leave_dict(builder);
+		l_dbus_message_builder_leave_array(builder);
+		l_dbus_message_builder_enter_array(builder, "s");
+		l_dbus_message_builder_leave_array(builder);
+
+		l_dbus_message_builder_finalize(builder);
+		l_dbus_message_builder_destroy(builder);
+		l_dbus_send(dbus, signal);
+	}
 }
 
 static void set_property_complete(struct l_dbus *dbus,
@@ -1126,3 +1167,232 @@ bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
 
 	return true;
 }
+
+static struct l_dbus_message *properties_get(struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	const struct interface_instance *instance;
+	const char *interface_name, *property_name, *signature;
+	const struct _dbus_property *property;
+	struct _dbus_object_tree *tree = _dbus_get_tree(dbus);
+	const struct object_node *object;
+	struct l_dbus_message *reply;
+	struct l_dbus_message_builder *builder;
+
+	if (!l_dbus_message_get_arguments(message, "ss", &interface_name,
+						&property_name))
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Invalid arguments");
+
+	object = l_hashmap_lookup(tree->objects,
+					l_dbus_message_get_path(message));
+	/* If we got here the object must exist */
+
+	instance = l_queue_find(object->instances,
+				match_interface_instance,
+				(char *) interface_name);
+	if (!instance)
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Object has no interface %s",
+						interface_name);
+
+	property = _dbus_interface_find_property(instance->interface,
+							property_name);
+	if (!property)
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Unknown Property %s",
+						property_name);
+
+
+	reply = l_dbus_message_new_method_return(message);
+	builder = l_dbus_message_builder_new(reply);
+
+	signature = property->metainfo + strlen(property->metainfo) + 1;
+
+	l_dbus_message_builder_enter_variant(builder, signature);
+
+	if (property->getter(dbus, message, builder, instance->user_data))
+		l_dbus_message_builder_leave_variant(builder);
+	else {
+		l_dbus_message_unref(reply);
+
+		reply = l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"Failed",
+						"Getting property value "
+						"failed");
+	}
+
+	l_dbus_message_builder_finalize(builder);
+	l_dbus_message_builder_destroy(builder);
+
+	return reply;
+}
+
+static void properties_set_complete(struct l_dbus *dbus,
+					struct l_dbus_message *message,
+					struct l_dbus_message *error)
+{
+	struct l_dbus_message *reply;
+	const char *interface_name, *property_name;
+	struct l_dbus_message_iter variant;
+
+	if (error) {
+		l_dbus_message_unref(message);
+
+		l_dbus_send(dbus, error);
+
+		return;
+	}
+
+	reply = l_dbus_message_new_method_return(message);
+	l_dbus_message_set_arguments(reply, "");
+	l_dbus_send(dbus, reply);
+
+	l_dbus_message_get_arguments(message, "ssv", &interface_name,
+					&property_name, &variant);
+
+	_dbus_object_tree_property_changed(dbus,
+					l_dbus_message_get_path(message),
+					interface_name, property_name,
+					&variant);
+
+	l_dbus_message_unref(message);
+}
+
+static struct l_dbus_message *properties_set(struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	struct l_dbus_interface *interface;
+	const struct interface_instance *instance;
+	const char *interface_name, *property_name;
+	const struct _dbus_property *property;
+	struct l_dbus_message_iter variant;
+	struct _dbus_object_tree *tree = _dbus_get_tree(dbus);
+	const struct object_node *object;
+
+	if (!l_dbus_message_get_arguments(message, "ssv", &interface_name,
+						&property_name, &variant))
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Invalid arguments");
+
+	interface = l_hashmap_lookup(tree->interfaces, interface_name);
+	if (!interface)
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Unknown Interface %s",
+						interface_name);
+
+	property = _dbus_interface_find_property(interface, property_name);
+	if (!property)
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Unknown Property %s",
+						property_name);
+
+	if (!property->setter)
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Property %s is read-only",
+						property_name);
+
+	object = l_hashmap_lookup(tree->objects,
+					l_dbus_message_get_path(message));
+	/* If we got here the object must exist */
+
+	instance = l_queue_find(object->instances,
+				match_interface_instance,
+				(char *) interface_name);
+	if (!instance)
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Object has no interface %s",
+						interface_name);
+
+	property->setter(dbus, l_dbus_message_ref(message), &variant,
+				properties_set_complete, instance->user_data);
+
+	return NULL;
+}
+
+static struct l_dbus_message *properties_get_all(struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	const struct interface_instance *instance;
+	const char *interface_name;
+	struct _dbus_object_tree *tree = _dbus_get_tree(dbus);
+	const struct object_node *object;
+	struct l_dbus_message *reply;
+	struct l_dbus_message_builder *builder;
+
+	if (!l_dbus_message_get_arguments(message, "s", &interface_name))
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Invalid arguments");
+
+	object = l_hashmap_lookup(tree->objects,
+					l_dbus_message_get_path(message));
+	/* If we got here the object must exist */
+
+	instance = l_queue_find(object->instances,
+				match_interface_instance,
+				(char *) interface_name);
+	if (!instance)
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"InvalidArgs",
+						"Object has no interface %s",
+						interface_name);
+
+	reply = l_dbus_message_new_method_return(message);
+	builder = l_dbus_message_builder_new(reply);
+
+	if (!get_properties_dict(dbus, message, builder, instance->interface,
+					instance->user_data)) {
+		l_dbus_message_unref(reply);
+
+		reply = l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"Failed",
+						"Getting property values "
+						"failed");
+	}
+
+	l_dbus_message_builder_finalize(builder);
+	l_dbus_message_builder_destroy(builder);
+
+	return reply;
+}
+
+static void properties_setup_func(struct l_dbus_interface *interface)
+{
+	l_dbus_interface_method(interface, "Get", 0,
+				properties_get, "v", "ss",
+				"value", "interface_name", "property_name");
+	l_dbus_interface_method(interface, "Set", 0,
+				properties_set, "", "ssv",
+				"interface_name", "property_name", "value");
+	l_dbus_interface_method(interface, "GetAll", 0,
+				properties_get_all, "a{sv}", "s",
+				"props", "interface_name");
+
+	l_dbus_interface_signal(interface, "PropertiesChanged", 0, "sa{sv}as",
+				"interface_name", "changed_properties",
+				"invalidated_properties");
+}
diff --git a/ell/dbus.c b/ell/dbus.c
index 796240f..873a8c2 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -51,7 +51,6 @@
 
 #define DBUS_INTERFACE_DBUS		"org.freedesktop.DBus"
 #define DBUS_INTERFACE_INTROSPECTABLE	"org.freedesktop.DBus.Introspectable"
-#define DBUS_INTERFACE_PROPERTIES	"org.freedesktop.DBus.Properties"
 
 #define DBUS_MAXIMUM_MATCH_RULE_LENGTH	1024
 
diff --git a/examples/dbus-service.c b/examples/dbus-service.c
index 8abeba4..b974f37 100644
--- a/examples/dbus-service.c
+++ b/examples/dbus-service.c
@@ -234,6 +234,13 @@ int main(int argc, char *argv[])
 		goto cleanup;
 	}
 
+	if (!l_dbus_object_add_interface(dbus, "/test",
+				"org.freedesktop.DBus.Properties", NULL)) {
+		l_info("Unable to instantiate the properties interface");
+		test_data_destroy(test);
+		goto cleanup;
+	}
+
 	l_main_run();
 
 	l_dbus_unregister_object(dbus, "/test");
-- 
2.5.0

[PATCH 8/8] dbus: Implement org.freedesktop.DBus.ObjectManager

[Andrew Zaborowski]

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

---
 ell/dbus-service.c      | 223 +++++++++++++++++++++++++++++++++++++++++++++++-
 ell/dbus.c              |  14 +++
 ell/dbus.h              |   2 +
 examples/dbus-service.c |   5 ++
 4 files changed, 242 insertions(+), 2 deletions(-)

diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 2b38e0a..fcdf9f9 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -27,6 +27,8 @@
 #define _GNU_SOURCE
 #include <stdarg.h>
 #include <string.h>
+#include <stdio.h>
+#include <stdlib.h>
 
 #include "util.h"
 #include "queue.h"
@@ -48,6 +50,7 @@ static const char *static_introspectable =
 		"\t\t</method>\n\t</interface>\n";
 
 #define DBUS_INTERFACE_PROPERTIES	"org.freedesktop.DBus.Properties"
+#define DBUS_INTERFACE_OBJECT_MANAGER	"org.freedesktop.DBus.ObjectManager"
 
 struct _dbus_method {
 	l_dbus_interface_method_cb_t cb;
@@ -98,10 +101,16 @@ struct object_node {
 	void (*destroy) (void *);
 };
 
+struct object_manager {
+	char *path;
+	struct l_dbus *dbus;
+};
+
 struct _dbus_object_tree {
 	struct l_hashmap *interfaces;
 	struct l_hashmap *objects;
 	struct object_node *root;
+	struct l_queue *object_managers;
 };
 
 void _dbus_method_introspection(struct _dbus_method *info,
@@ -512,6 +521,7 @@ static bool match_interface_instance(const void *a, const void *b)
 }
 
 static void properties_setup_func(struct l_dbus_interface *);
+static void object_manager_setup_func(struct l_dbus_interface *);
 
 struct _dbus_object_tree *_dbus_object_tree_new()
 {
@@ -532,6 +542,13 @@ struct _dbus_object_tree *_dbus_object_tree_new()
 						properties_setup_func, NULL,
 						false);
 
+	tree->object_managers = l_queue_new();
+
+	_dbus_object_tree_register_interface(tree,
+						DBUS_INTERFACE_OBJECT_MANAGER,
+						object_manager_setup_func, NULL,
+						false);
+
 	return tree;
 }
 
@@ -556,6 +573,14 @@ static void subtree_free(struct object_node *node)
 	l_free(node);
 }
 
+static void object_manager_free(void *data)
+{
+	struct object_manager *manager = data;
+
+	l_free(manager->path);
+	l_free(manager);
+}
+
 void _dbus_object_tree_free(struct _dbus_object_tree *tree)
 {
 	l_hashmap_destroy(tree->interfaces,
@@ -564,6 +589,8 @@ void _dbus_object_tree_free(struct _dbus_object_tree *tree)
 
 	subtree_free(tree->root);
 
+	l_queue_destroy(tree->object_managers, object_manager_free);
+
 	l_free(tree);
 }
 
@@ -1010,6 +1037,44 @@ bool _dbus_object_tree_unregister_interface(struct _dbus_object_tree *tree,
 	return true;
 }
 
+static bool build_interfaces_added_signal(const struct object_manager *manager,
+				const char *path,
+				const struct interface_instance *instance)
+{
+	struct l_dbus_message *signal;
+	struct l_dbus_message_builder *builder;
+
+	signal = l_dbus_message_new_signal(manager->dbus, manager->path,
+						DBUS_INTERFACE_OBJECT_MANAGER,
+						"InterfacesAdded");
+	builder = l_dbus_message_builder_new(signal);
+
+	l_dbus_message_builder_append_basic(builder, 'o', path);
+	l_dbus_message_builder_enter_array(builder, "{sa{sv}}");
+	l_dbus_message_builder_enter_dict(builder, "sa{sv}");
+	l_dbus_message_builder_append_basic(builder, 's',
+						instance->interface->name);
+
+	if (!get_properties_dict(manager->dbus, signal, builder,
+					instance->interface,
+					instance->user_data)) {
+		l_dbus_message_builder_destroy(builder);
+		l_dbus_message_unref(signal);
+
+		return false;
+	}
+
+	l_dbus_message_builder_leave_dict(builder);
+	l_dbus_message_builder_leave_array(builder);
+
+	l_dbus_message_builder_finalize(builder);
+	l_dbus_message_builder_destroy(builder);
+
+	l_dbus_send(manager->dbus, signal);
+
+	return true;
+}
+
 bool _dbus_object_tree_add_interface(struct _dbus_object_tree *tree,
 					const char *path, const char *interface,
 					void *user_data)
@@ -1017,6 +1082,9 @@ bool _dbus_object_tree_add_interface(struct _dbus_object_tree *tree,
 	struct object_node *object;
 	struct l_dbus_interface *dbi;
 	struct interface_instance *instance;
+	const struct l_queue_entry *entry;
+	struct object_manager *manager;
+	size_t path_len;
 
 	dbi = l_hashmap_lookup(tree->interfaces, interface);
 	if (!dbi)
@@ -1044,15 +1112,47 @@ bool _dbus_object_tree_add_interface(struct _dbus_object_tree *tree,
 
 	l_queue_push_tail(object->instances, instance);
 
+	for (entry = l_queue_get_entries(tree->object_managers); entry;
+			entry = entry->next) {
+		manager = entry->data;
+		path_len = strlen(manager->path);
+
+		if (memcmp(path, manager->path, path_len) ||
+				(path[path_len] != '\0' &&
+				 path[path_len] != '/'))
+			continue;
+
+		build_interfaces_added_signal(manager, path, instance);
+	}
+
+	if (!strcmp(interface, DBUS_INTERFACE_OBJECT_MANAGER)) {
+		manager = l_new(struct object_manager, 1);
+		manager->path = l_strdup(path);
+		manager->dbus = instance->user_data;
+
+		l_queue_push_tail(tree->object_managers, manager);
+	}
+
 	return true;
 }
 
+static bool match_object_manager_path(const void *a, const void *b)
+{
+	const struct object_manager *manager = a;
+
+	return !strcmp(manager->path, b);
+}
+
 bool _dbus_object_tree_remove_interface(struct _dbus_object_tree *tree,
-					const char *path,
-					const char *interface)
+					const char *path, const char *interface)
 {
 	struct object_node *node;
 	struct interface_instance *instance;
+	const struct l_queue_entry *entry;
+	struct object_manager *manager;
+	struct l_dbus_message *signal;
+	struct l_dbus_message_builder *builder;
+	size_t path_len;
 
 	node = l_hashmap_lookup(tree->objects, path);
 	if (!node)
@@ -1065,6 +1165,39 @@ bool _dbus_object_tree_remove_interface(struct _dbus_object_tree *tree,
 
 	interface_instance_free(instance);
 
+	if (!strcmp(interface, DBUS_INTERFACE_OBJECT_MANAGER)) {
+		manager = l_queue_remove_if(tree->object_managers,
+						match_object_manager_path,
+						(char *) path);
+
+		object_manager_free(manager);
+	}
+
+	for (entry = l_queue_get_entries(tree->object_managers); entry;
+			entry = entry->next) {
+		manager = entry->data;
+		path_len = strlen(manager->path);
+
+		if (memcmp(path, manager->path, path_len) ||
+				(path[path_len] != '\0' &&
+				 path[path_len] != '/'))
+			continue;
+
+		signal = l_dbus_message_new_signal(manager->dbus, manager->path,
+						DBUS_INTERFACE_OBJECT_MANAGER,
+						"InterfacesRemoved");
+
+		builder = l_dbus_message_builder_new(signal);
+
+		l_dbus_message_builder_append_basic(builder, 'o', path);
+		l_dbus_message_builder_enter_array(builder, "s");
+		l_dbus_message_builder_append_basic(builder, 's', interface);
+		l_dbus_message_builder_leave_array(builder);
+		l_dbus_message_builder_finalize(builder);
+		l_dbus_message_builder_destroy(builder);
+		l_dbus_send(manager->dbus, signal);
+	}
+
 	return true;
 }
 
@@ -1396,3 +1529,89 @@ static void properties_setup_func(struct l_dbus_interface *interface)
 				"interface_name", "changed_properties",
 				"invalidated_properties");
 }
+
+static void collect_objects(struct l_dbus *dbus, struct l_dbus_message *message,
+				struct l_dbus_message_builder *builder,
+				const struct object_node *node,
+				const char *path)
+{
+	const struct l_queue_entry *entry;
+	const struct child_node *child;
+	char *child_path;
+	const struct interface_instance *instance;
+
+	if (!node->instances)
+		goto recurse;
+
+	l_dbus_message_builder_enter_dict(builder, "oa{sa{sv}}");
+	l_dbus_message_builder_append_basic(builder, 'o', path);
+	l_dbus_message_builder_enter_array(builder, "{sa{sv}}");
+
+	for (entry = l_queue_get_entries(node->instances); entry;
+			entry = entry->next) {
+		instance = entry->data;
+
+		l_dbus_message_builder_enter_dict(builder, "sa{sv}");
+		l_dbus_message_builder_append_basic(builder, 's',
+						instance->interface->name);
+		get_properties_dict(dbus, message, builder, instance->interface,
+					instance->user_data);
+		l_dbus_message_builder_leave_dict(builder);
+	}
+
+	l_dbus_message_builder_leave_array(builder);
+	l_dbus_message_builder_leave_dict(builder);
+
+recurse:
+	if (!strcmp(path, "/"))
+		path = "";
+
+	for (child = node->children; child; child = child->next) {
+		child_path = l_strdup_printf("%s/%s", path, child->subpath);
+
+		collect_objects(dbus, message, builder,
+				child->node, child_path);
+
+		l_free(child_path);
+	}
+}
+
+static struct l_dbus_message *get_managed_objects(struct l_dbus *dbus,
+						struct l_dbus_message *message,
+						void *user_data)
+{
+	struct _dbus_object_tree *tree = _dbus_get_tree(dbus);
+	const struct object_node *node;
+	struct l_dbus_message *reply;
+	struct l_dbus_message_builder *builder;
+	const char *path;
+
+	path = l_dbus_message_get_path(message);
+	node = l_hashmap_lookup(tree->objects, path);
+
+	reply = l_dbus_message_new_method_return(message);
+	builder = l_dbus_message_builder_new(reply);
+
+	l_dbus_message_builder_enter_array(builder, "{oa{sa{sv}}}");
+
+	collect_objects(dbus, message, builder, node, path);
+
+	l_dbus_message_builder_leave_array(builder);
+
+	l_dbus_message_builder_finalize(builder);
+	l_dbus_message_builder_destroy(builder);
+
+	return reply;
+}
+
+static void object_manager_setup_func(struct l_dbus_interface *interface)
+{
+	l_dbus_interface_method(interface, "GetManagedObjects", 0,
+				get_managed_objects, "a{oa{sa{sv}}}", "",
+				"objpath_interfaces_and_properties");
+
+	l_dbus_interface_signal(interface, "InterfacesAdded", 0, "oa{sa{sv}}",
+				"object_path", "interfaces_and_properties");
+	l_dbus_interface_signal(interface, "InterfacesRemoved", 0, "oas",
+				"object_path", "interfaces");
+}
diff --git a/ell/dbus.c b/ell/dbus.c
index 873a8c2..8c9318a 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -51,6 +51,7 @@
 
 #define DBUS_INTERFACE_DBUS		"org.freedesktop.DBus"
 #define DBUS_INTERFACE_INTROSPECTABLE	"org.freedesktop.DBus.Introspectable"
+#define DBUS_INTERFACE_OBJECT_MANAGER	"org.freedesktop.DBus.ObjectManager"
 
 #define DBUS_MAXIMUM_MATCH_RULE_LENGTH	1024
 
@@ -1380,6 +1381,19 @@ LIB_EXPORT bool l_dbus_object_remove_interface(struct l_dbus *dbus,
 							interface);
 }
 
+LIB_EXPORT bool l_dbus_object_manager_enable(struct l_dbus *dbus)
+{
+	if (unlikely(!dbus))
+		return false;
+
+	if (unlikely(!dbus->tree))
+		return false;
+
+	return _dbus_object_tree_add_interface(dbus->tree, "/",
+						DBUS_INTERFACE_OBJECT_MANAGER,
+						dbus->tree);
+}
+
 void _dbus1_filter_format_match(struct dbus1_filter_data *data, char *rule,
 					size_t size)
 {
diff --git a/ell/dbus.h b/ell/dbus.h
index 29253b6..84bf7a4 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -211,6 +211,8 @@ bool l_dbus_object_add_interface(struct l_dbus *dbus, const char *object,
 bool l_dbus_object_remove_interface(struct l_dbus *dbus, const char *object,
 					const char *interface);
 
+bool l_dbus_object_manager_enable(struct l_dbus *dbus);
+
 unsigned int l_dbus_add_disconnect_watch(struct l_dbus *dbus,
 					const char *name,
 					l_dbus_watch_func_t disconnect_func,
diff --git a/examples/dbus-service.c b/examples/dbus-service.c
index b974f37..964d735 100644
--- a/examples/dbus-service.c
+++ b/examples/dbus-service.c
@@ -217,6 +217,11 @@ int main(int argc, char *argv[])
 				request_name_setup,
 				request_name_callback, NULL, NULL);
 
+	if (!l_dbus_object_manager_enable(dbus)) {
+		l_info("Unable to enable Object Manager");
+		goto cleanup;
+	}
+
 	test = l_new(struct test_data, 1);
 	test->string = l_strdup("Default");
 	test->integer = 42;
-- 
2.5.0

Re: [PATCH 1/8] dbus: More complete checks when removing nodes.

[Denis Kenzior]

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

Hi Andrew,

On 02/04/2016 08:58 PM, Andrew Zaborowski wrote:
> This addresses two problems with nodes being freed from the object tree
> but not from tree->objects.
> ---
>   ell/dbus-service.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)

Applied, thanks.

Regards,
-Denis

[PATCH 1/8] dbus: More complete checks when removing nodes.

[Andrew Zaborowski]

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

This addresses two problems with nodes being freed from the object tree
but not from tree->objects.

With the current semantics for _dbus_object_tree_prune_node to not
incorrectly remove parent nodes that are still in tree->objects it
would be sufficient to check if the node has any interface instances
left. But with the ongoing ObjectManager it's better to directly check
the path against tree->objects.
---
 ell/dbus-private.h |  4 ++-
 ell/dbus-service.c | 72 ++++++++++++++++++++++++++++++++++++++++--------------
 2 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index e60d09e..a0ddb35 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -167,7 +167,9 @@ struct object_node *_dbus_object_tree_makepath(struct _dbus_object_tree *tree,
 						const char *path);
 struct object_node *_dbus_object_tree_lookup(struct _dbus_object_tree *tree,
 						const char *path);
-void _dbus_object_tree_prune_node(struct object_node *node);
+bool _dbus_object_tree_prune_node(struct _dbus_object_tree *tree,
+					struct object_node *node,
+					const char *path);
 
 bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
 				const char *path, const char *interface,
diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index e082226..9380740 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -636,33 +636,65 @@ struct object_node *_dbus_object_tree_lookup(struct _dbus_object_tree *tree,
 	return lookup_recurse(tree->root, path);
 }
 
-void _dbus_object_tree_prune_node(struct object_node *node)
+bool _dbus_object_tree_prune_node(struct _dbus_object_tree *tree,
+					struct object_node *node,
+					const char *path)
 {
-	struct object_node *parent = node->parent;
-	struct child_node *p = NULL, *c;
+	struct object_node *parent;
+	struct child_node *p, *c;
+	struct interface_instance *instance;
+	char parentpath[strlen(path) + 1];
+
+	if (!node) {
+		node = l_hashmap_remove(tree->objects, path);
+		if (!node)
+			return false;
+	}
+
+	while ((instance = l_queue_pop_head(node->instances)))
+		interface_instance_free(instance);
+
+	if (node->children || !node->parent)
+		return true;
 
-	while (parent) {
-		for (c = parent->children, p = NULL; c; p = c, c = c->next) {
-			if (c->node != node)
-				continue;
+	/*
+	 * Walk up the tree until a node that either has more than one
+	 * child, is the root or is in the objects hashmap.
+	 */
+	strcpy(parentpath, path);
 
-			if (p)
-				p->next = c->next;
-			else
-				parent->children = c->next;
+	while (true) {
+		parent = node->parent;
 
-			subtree_free(c->node);
-			l_free(c);
+		if (parent == tree->root)
+			break;
 
+		if (parent->children->next)
 			break;
-		}
 
-		if (parent->children != NULL)
-			return;
+		/* Parent's path */
+		parentpath[strlen(parentpath) -
+			strlen(parent->children->subpath) - 1] = '\0';
+
+		if (l_hashmap_lookup(tree->objects, parentpath))
+			break;
 
 		node = parent;
-		parent = node->parent;
 	}
+
+	for (c = parent->children, p = NULL; c; p = c, c = c->next)
+		if (c->node == node)
+			break;
+
+	if (p)
+		p->next = c->next;
+	else
+		parent->children = c->next;
+
+	l_free(c);
+	subtree_free(node);
+
+	return true;
 }
 
 bool _dbus_object_tree_register(struct _dbus_object_tree *tree,
@@ -741,9 +773,11 @@ bool _dbus_object_tree_unregister(struct _dbus_object_tree *tree,
 	if (instance)
 		interface_instance_free(instance);
 
-	if (l_queue_isempty(node->instances) && !node->children) {
+	if (l_queue_isempty(node->instances)) {
 		l_hashmap_remove(tree->objects, path);
-		_dbus_object_tree_prune_node(node);
+
+		if (!node->children)
+			_dbus_object_tree_prune_node(tree, node, path);
 	}
 
 	return r;
-- 
2.5.0