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