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

[Andrew Zaborowski <andrew.zaborowski@intel.com>]

[-- 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