[PATCH] dbus: Fix read after free during cleanup

[PATCH] dbus: Fix read after free during cleanup

[Andrew Zaborowski]

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

subtree_free will be checking if interfaces of objects being freed have
a destroy callback so the interfaces should not be freed before that.
---
 ell/dbus-service.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 5d49836..1e3bb66 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -583,12 +583,12 @@ static void object_manager_free(void *data)
 
 void _dbus_object_tree_free(struct _dbus_object_tree *tree)
 {
+	subtree_free(tree->root);
+
 	l_hashmap_destroy(tree->interfaces,
 			(l_hashmap_destroy_func_t) _dbus_interface_free);
 	l_hashmap_destroy(tree->objects, NULL);
 
-	subtree_free(tree->root);
-
 	l_queue_destroy(tree->object_managers, object_manager_free);
 
 	l_free(tree);
-- 
2.5.0

[PATCH 1/9] 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 39f926b..d506cd3 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 2/9] dbus: Private function to retrieve the tree for a connection.

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 985 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 e60d09e..35c97b7 100644
--- a/ell/dbus-private.h
+++ b/ell/dbus-private.h
@@ -212,6 +212,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 1d9aed5..14849e8 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;
+}
+
 LIB_EXPORT bool l_dbus_register_interface(struct l_dbus *dbus,
 				const char *path, const char *interface,
 				l_dbus_interface_setup_func_t setup_func,
-- 
2.5.0

[PATCH 3/9] dbus: setters and getters API for properties.

[Andrew Zaborowski]

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

---
 ell/dbus-service.c | 34 +++++++++++++++-------------------
 ell/dbus-service.h | 29 +++++++++++++++++++++--------
 2 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 639872b..81036e5 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;
@@ -853,3 +843,9 @@ bool _dbus_object_tree_dispatch(struct _dbus_object_tree *tree,
 
 	return true;
 }
+
+LIB_EXPORT void l_dbus_property_changed(struct l_dbus *dbus, const char *path,
+					const char *interface,
+					const char *property)
+{
+}
diff --git a/ell/dbus-service.h b/ell/dbus-service.h
index ce53082..619d9a6 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,12 @@ 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);
+
+void l_dbus_property_changed(struct l_dbus *dbus, const char *path,
+				const char *interface, const char *property);
 
 #ifdef __cplusplus
 }
-- 
2.5.0

[PATCH 4/9] dbus: Separate interface registration from instantiation

[Andrew Zaborowski]

[-- Attachment #1: Type: text/plain, Size: 17418 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 | 192 ++++++++++++++++++++++++++++++++++++++++-------------
 ell/dbus.c         | 147 ++++++++++++++++++++++++++++++++++++++--
 ell/dbus.h         |  19 ++++--
 4 files changed, 318 insertions(+), 62 deletions(-)

diff --git a/ell/dbus-private.h b/ell/dbus-private.h
index 35c97b7..8c58218 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 81036e5..564b90f 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,171 @@ 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;
+
+	/*
+	 * Registered objects in the tree are marked by being present in the
+	 * tree->objects hash and having non-null node->instances.  Remaining
+	 * nodes are intermediate path elements added and removed
+	 * automatically.
+	 */
+	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 +836,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 14849e8..796240f 100644
--- a/ell/dbus.c
+++ b/ell/dbus.c
@@ -1219,11 +1219,28 @@ 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*
+ * @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;
@@ -1231,12 +1248,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))
@@ -1245,7 +1262,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 d506cd3..29253b6 100644
--- a/ell/dbus.h
+++ b/ell/dbus.h
@@ -195,13 +195,22 @@ bool l_dbus_message_builder_append_from_iter(
 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,
-- 
2.5.0

[PATCH 5/9] dbus: Handle legacy GetProperties and SetProperty automatically.

[Andrew Zaborowski]

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

In this version, for technical reasons, get_properties_dict just
returns false if any getter fails, and the builder is left in an
undefined state so the caller must return an error rather than
leaving out the property that failed (applies for .GetProperties,
Properties.GetAll, ObjectManager.GetManagedObjects and the
.InterfacesAdded signal).
---
 ell/dbus-private.h |   6 ++
 ell/dbus-service.c | 208 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 214 insertions(+)

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 564b90f..01a60e3 100644
--- a/ell/dbus-service.c
+++ b/ell/dbus-service.c
@@ -718,6 +718,199 @@ 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;
+	const struct _dbus_property *property;
+	const char *signature;
+	const struct interface_instance *instance;
+	const struct object_node *object;
+	struct l_dbus_message_iter value;
+	struct _dbus_object_tree *tree = _dbus_get_tree(dbus);
+	bool r;
+
+	object = l_hashmap_lookup(tree->objects, path);
+
+	instance = l_queue_find(object->instances, match_interface_instance,
+				interface_name);
+
+	property = _dbus_interface_find_property(instance->interface,
+							property_name);
+
+	signature = property->metainfo + strlen(property->metainfo) + 1;
+
+	if (instance->interface->handle_old_style_properties) {
+		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);
+
+		if (variant) {
+			memcpy(&value, variant, sizeof(value));
+			r = l_dbus_message_builder_append_from_iter(builder,
+								&value);
+		} else {
+			r = property->getter(dbus, signal, builder,
+						instance->user_data);
+		}
+
+		l_dbus_message_builder_leave_variant(builder);
+
+		l_dbus_message_builder_finalize(builder);
+		l_dbus_message_builder_destroy(builder);
+		if (r)
+			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;
+
+	l_dbus_message_builder_enter_array(builder, "{sv}");
+
+	for (entry = l_queue_get_entries(interface->properties); entry;
+			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);
+
+		if (!property->getter(dbus, message, builder, user_data))
+			return false;
+
+		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");
+	} else
+		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 *),
@@ -741,6 +934,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);
@@ -948,4 +1154,6 @@ LIB_EXPORT void l_dbus_property_changed(struct l_dbus *dbus, const char *path,
 					const char *interface,
 					const char *property)
 {
+	_dbus_object_tree_property_changed(dbus, path, interface, property,
+						NULL);
 }
-- 
2.5.0

[PATCH 6/9] dbus: Implement org.freedesktop.DBus.Properties

[Andrew Zaborowski]

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

---
 ell/dbus-service.c | 275 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 ell/dbus.c         |   1 -
 2 files changed, 275 insertions(+), 1 deletion(-)

diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 01a60e3..577bbfb 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;
 }
 
@@ -718,6 +726,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,
@@ -771,6 +780,43 @@ void _dbus_object_tree_property_changed(struct l_dbus *dbus,
 		if (r)
 			l_dbus_send(dbus, signal);
 	}
+
+	if (l_queue_find(object->instances, match_interface_instance,
+				DBUS_INTERFACE_PROPERTIES)) {
+		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);
+
+		if (variant) {
+			memcpy(&value, variant, sizeof(value));
+			r = l_dbus_message_builder_append_from_iter(builder,
+								&value);
+		} else {
+			r = property->getter(dbus, signal, builder,
+						instance->user_data);
+		}
+
+		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);
+		if (r)
+			l_dbus_send(dbus, signal);
+	}
 }
 
 static void set_property_complete(struct l_dbus *dbus,
@@ -1157,3 +1203,232 @@ LIB_EXPORT void l_dbus_property_changed(struct l_dbus *dbus, const char *path,
 	_dbus_object_tree_property_changed(dbus, path, interface, property,
 						NULL);
 }
+
+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");
+	} else
+		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
 
-- 
2.5.0

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

[Andrew Zaborowski]

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

---
 ell/dbus-service.c | 242 ++++++++++++++++++++++++++++++++++++++++++++++++++++-
 ell/dbus.c         |  14 ++++
 ell/dbus.h         |   2 +
 3 files changed, 256 insertions(+), 2 deletions(-)

diff --git a/ell/dbus-service.c b/ell/dbus-service.c
index 577bbfb..5d49836 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);
 }
 
@@ -1038,6 +1065,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)
@@ -1045,6 +1110,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)
@@ -1072,15 +1140,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] != '/' && path_len > 1))
+			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)
@@ -1093,6 +1193,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] != '/' && path_len > 1))
+			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;
 }
 
@@ -1432,3 +1565,108 @@ static void properties_setup_func(struct l_dbus_interface *interface)
 				"interface_name", "changed_properties",
 				"invalidated_properties");
 }
+
+static bool 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;
+	bool r;
+
+	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);
+
+		if (!get_properties_dict(dbus, message, builder,
+						instance->interface,
+						instance->user_data))
+			return false;
+
+		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);
+
+		r = collect_objects(dbus, message, builder,
+					child->node, child_path);
+
+		l_free(child_path);
+
+		if (!r)
+			return false;
+	}
+
+	return true;
+}
+
+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}}}");
+
+	if (!collect_objects(dbus, message, builder, node, path)) {
+		l_dbus_message_builder_destroy(builder);
+		l_dbus_message_unref(reply);
+
+		return l_dbus_message_new_error(message,
+						"org.freedesktop.DBus.Error."
+						"Failed",
+						"Getting property values "
+						"failed");
+	}
+
+	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..153bd0b 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);
+}
+
 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,
-- 
2.5.0

[PATCH 8/9] examples: update dbus-service to new API

[Andrew Zaborowski]

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

---
 examples/dbus-service.c | 168 +++++++++++++++++++++++++-----------------------
 1 file changed, 86 insertions(+), 82 deletions(-)

diff --git a/examples/dbus-service.c b/examples/dbus-service.c
index 40c1646..964d735 100644
--- a/examples/dbus-service.c
+++ b/examples/dbus-service.c
@@ -98,111 +98,98 @@ static void test_data_destroy(void *data)
 	l_free(test);
 }
 
-static struct l_dbus_message *test_set_property(struct l_dbus *dbus,
+static struct l_dbus_message *test_method_call(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);
+
+	l_info("Method Call");
 
 	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;
+	return reply;
 }
 
-static struct l_dbus_message *test_get_properties(struct l_dbus *dbus,
-						struct l_dbus_message *message,
-						void *user_data)
+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;
-	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);
+	l_dbus_message_builder_append_basic(builder, 's', test->string);
 
-	return reply;
+	return true;
 }
 
-static struct l_dbus_message *test_method_call(struct l_dbus *dbus,
-						struct l_dbus_message *message,
-						void *user_data)
+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)
 {
-	struct l_dbus_message *reply;
+	const char *strvalue;
+	struct test_data *test = user_data;
 
-	l_info("Method Call");
+	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;
+	}
 
-	reply = l_dbus_message_new_method_return(message);
-	l_dbus_message_set_arguments(reply, "");
+	l_info("New String value: %s", strvalue);
+	l_free(test->string);
+	test->string = l_strdup(strvalue);
 
-	return reply;
+	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,
-				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_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[])
@@ -230,21 +217,38 @@ 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;
 
-	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, true)) {
 		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;
+	}
+
+	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_interface(dbus, "/test", "org.test");
+	l_dbus_unregister_object(dbus, "/test");
 
 cleanup:
 	l_dbus_destroy(dbus);
-- 
2.5.0

[PATCH 9/9] test: update to new dbus API

[Andrew Zaborowski]

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

---
 unit/test-dbus-service.c | 35 ++++++++++++++++++++++++++++-------
 unit/test-kdbus.c        |  9 +++++++--
 2 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/unit/test-dbus-service.c b/unit/test-dbus-service.c
index 7452269..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",
@@ -315,6 +318,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 +352,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);
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

Re: [PATCH] dbus: Fix read after free during cleanup

[Denis Kenzior]

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

Hi Andrew,

On 02/10/2016 04:45 AM, Andrew Zaborowski wrote:
> subtree_free will be checking if interfaces of objects being freed have
> a destroy callback so the interfaces should not be freed before that.
> ---
>   ell/dbus-service.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>

Applied, thanks.

Regards,
-Denis

Re: [PATCH 1/9] dbus: Message builder function to copy from an iter.

[Denis Kenzior]

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

Hi Andrew,

On 02/10/2016 04:45 AM, Andrew Zaborowski wrote:
> ---
>   ell/dbus-message.c | 125 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ell/dbus.h         |   4 ++
>   2 files changed, 129 insertions(+)
>

All 9 patches have been applied, thanks.

Regards,
-Denis

Re: [PATCH 4/9] dbus: Separate interface registration from instantiation

[Denis Kenzior]

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

Hi Andrew,

On 02/10/2016 04:45 AM, Andrew Zaborowski wrote:
> 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 | 192 ++++++++++++++++++++++++++++++++++++++++-------------
>   ell/dbus.c         | 147 ++++++++++++++++++++++++++++++++++++++--
>   ell/dbus.h         |  19 ++++--
>   4 files changed, 318 insertions(+), 62 deletions(-)
>

<snip>

> diff --git a/ell/dbus.c b/ell/dbus.c
> index 14849e8..796240f 100644
> --- a/ell/dbus.c
> +++ b/ell/dbus.c

<snip>

> +
> +LIB_EXPORT bool l_dbus_object_remove_interface(struct l_dbus *dbus,
> +						const char *object,
> +						const char *interface)

You forgot to add documentation for this one :)

> +{
> +	if (unlikely(!dbus))
> +		return false;
> +
> +	if (unlikely(!dbus->tree))
> +		return false;
> +
> +	return _dbus_object_tree_remove_interface(dbus->tree, object,
> +							interface);
>   }
>

Regards,
-Denis