chiark / gitweb /
sd-bus: make introspection data non-recursive
authorDavid Herrmann <dh.herrmann@gmail.com>
Sat, 5 Sep 2015 17:43:29 +0000 (19:43 +0200)
committerSven Eden <yamakuzure@gmx.net>
Tue, 14 Mar 2017 09:19:06 +0000 (10:19 +0100)
Currently, our introspection data looks like this:

        <node>
         <interface name="org.freedesktop.DBus.Peer">
           ...
         </interface>
         <interface name="org.freedesktop.DBus.Introspectable">
           ...
         </interface>
         <interface name="org.freedesktop.DBus.Properties">
           ...
         </interface>
         <node name="org"/>
         <node name="org/freedesktop"/>
         <node name="org/freedesktop/login1"/>
         <node name="org/freedesktop/login1/user"/>
         <node name="org/freedesktop/login1/user/self"/>
         <node name="org/freedesktop/login1/user/_1000"/>
         <node name="org/freedesktop/login1/seat"/>
         <node name="org/freedesktop/login1/seat/self"/>
         <node name="org/freedesktop/login1/seat/seat0"/>
         <node name="org/freedesktop/login1/session"/>
         <node name="org/freedesktop/login1/session/self"/>
         <node name="org/freedesktop/login1/session/c1"/>
        </node>

(ordered alphabetically for better visibility)

This is grossly incorrect. The spec says that we're allowed to return
non-directed children, however, it does not allow us to return data
recursively in multiple parents. If we return "org", then we must not
return anything else that starts with "org/".

It is unclear, whether we can include child-nodes as a tree. Moreover, it
is usually not what the caller wants. Hence, this patch changes sd-bus to
never return introspection data recursively. Instead, only a single
child-layer is returned.

This patch relies on enumerators to never return hierarchies. If someone
registers an enumerator via sd_bus_add_enumerator, they better register
sub-enumerators if they support *TRUE* hierarchies. Each enumerator is
treated as a single layer and not filtered.
Enumerators are still allowed to return nested data. However, that data
is still required to be a single hierarchy. For instance, returning
"/org/foo" and "/com/bar" is fine, but including "/com" or "/org" in that
dataset is not.
This should be the default for enumerators and I see no reason to filter
in sd-bus. Moreover, filtering that data-set would require to sort the
strv by path and then do prefix-filtering. This is O(n log n), which
would be fine, but still better to avoid.

Fixes #664.

src/libelogind/sd-bus/bus-objects.c

index fa5c8ab..05a00f2 100644 (file)
@@ -169,11 +169,18 @@ static int add_enumerated_to_set(
         return 0;
 }
 
+enum {
+        /* if set, add_subtree() works recursively */
+        CHILDREN_RECURSIVE              = (1U << 1),
+        /* if set, add_subtree() scans object-manager hierarchies recursively */
+        CHILDREN_SUBHIERARCHIES         = (1U << 0),
+};
+
 static int add_subtree_to_set(
                 sd_bus *bus,
                 const char *prefix,
                 struct node *n,
-                bool skip_subhierarchies,
+                unsigned int flags,
                 Set *s,
                 sd_bus_error *error) {
 
@@ -205,8 +212,9 @@ static int add_subtree_to_set(
                 if (r < 0 && r != -EEXIST)
                         return r;
 
-                if (!skip_subhierarchies || !i->object_managers) {
-                        r = add_subtree_to_set(bus, prefix, i, skip_subhierarchies, s, error);
+                if ((flags & CHILDREN_RECURSIVE) &&
+                    ((flags & CHILDREN_SUBHIERARCHIES) || !i->object_managers)) {
+                        r = add_subtree_to_set(bus, prefix, i, flags, s, error);
                         if (r < 0)
                                 return r;
                         if (bus->nodes_modified)
@@ -221,7 +229,7 @@ static int get_child_nodes(
                 sd_bus *bus,
                 const char *prefix,
                 struct node *n,
-                bool skip_subhierarchies,
+                unsigned int flags,
                 Set **_s,
                 sd_bus_error *error) {
 
@@ -237,7 +245,7 @@ static int get_child_nodes(
         if (!s)
                 return -ENOMEM;
 
-        r = add_subtree_to_set(bus, prefix, n, skip_subhierarchies, s, error);
+        r = add_subtree_to_set(bus, prefix, n, flags, s, error);
         if (r < 0) {
                 set_free_free(s);
                 return r;
@@ -907,7 +915,7 @@ static int process_introspect(
         assert(n);
         assert(found_object);
 
-        r = get_child_nodes(bus, m->path, n, false, &s, &error);
+        r = get_child_nodes(bus, m->path, n, 0, &s, &error);
         if (r < 0)
                 return bus_maybe_reply_error(m, r, &error);
         if (bus->nodes_modified)
@@ -1173,7 +1181,7 @@ static int process_get_managed_objects(
         if (require_fallback || !n->object_managers)
                 return 0;
 
-        r = get_child_nodes(bus, m->path, n, true, &s, &error);
+        r = get_child_nodes(bus, m->path, n, CHILDREN_RECURSIVE, &s, &error);
         if (r < 0)
                 return r;
         if (bus->nodes_modified)