sd-bus: fix path of object-manager signals

Each signal of the ObjectManager interface carries the path of the object
in question as an argument. Therefore, a caller will deduce the object
this signal is generated for, by parsing the _argument_. A caller will
*not* use the object-path of the message itself (i.e., message->path).
This is done on purpose, so the caller can rely on message->path to be
the path of the actual object-manager that generated this signal, instead
of the path of the object that triggered this signal.

This commit fixes all InterfacesAdded/Removed signals to use the path of
the closest object-manager as message->path. 'closest' in this case means
closest parent with at least one object-manager registered.

This fix raises the question what happens if we stack object-managers in
a hierarchy. Two implementations are possible: First, we report each
object only on the nearest object-manager. Second, we report it on each
parent object-manager. This patch chooses the former. This is compatible
with other existing ObjectManager implementations, which are required to
call GetManagedObjects() recursively on each object they find, which
implements the ObjectManager interface.

diff --git a/src/libsystemd/sd-bus/bus-objects.c b/src/libsystemd/sd-bus/bus-objects.c
index a973bca..c25293e 100644 (file)
--- a/src/libsystemd/sd-bus/bus-objects.c
+++ b/src/libsystemd/sd-bus/bus-objects.c
@@ -173,6 +173,7 @@ static int add_subtree_to_set(
                 sd_bus *bus,
                 const char *prefix,
                 struct node *n,
+                bool skip_subhierarchies,
                 Set *s,
                 sd_bus_error *error) {
 
@@ -204,11 +205,13 @@ static int add_subtree_to_set(
                 if (r < 0 && r != -EEXIST)
                         return r;
 
-                r = add_subtree_to_set(bus, prefix, i, s, error);
-                if (r < 0)
-                        return r;
-                if (bus->nodes_modified)
-                        return 0;
+                if (!skip_subhierarchies || !i->object_managers) {
+                        r = add_subtree_to_set(bus, prefix, i, skip_subhierarchies, s, error);
+                        if (r < 0)
+                                return r;
+                        if (bus->nodes_modified)
+                                return 0;
+                }
         }
 
         return 0;
@@ -218,6 +221,7 @@ static int get_child_nodes(
                 sd_bus *bus,
                 const char *prefix,
                 struct node *n,
+                bool skip_subhierarchies,
                 Set **_s,
                 sd_bus_error *error) {
 
@@ -233,7 +237,7 @@ static int get_child_nodes(
         if (!s)
                 return -ENOMEM;
 
-        r = add_subtree_to_set(bus, prefix, n, s, error);
+        r = add_subtree_to_set(bus, prefix, n, skip_subhierarchies, s, error);
         if (r < 0) {
                 set_free_free(s);
                 return r;
@@ -900,7 +904,7 @@ static int process_introspect(
         assert(n);
         assert(found_object);
 
-        r = get_child_nodes(bus, m->path, n, &s, &error);
+        r = get_child_nodes(bus, m->path, n, false, &s, &error);
         if (r < 0)
                 return bus_maybe_reply_error(m, r, &error);
         if (bus->nodes_modified)
@@ -1166,7 +1170,7 @@ static int process_get_managed_objects(
         if (require_fallback || !n->object_managers)
                 return 0;
 
-        r = get_child_nodes(bus, m->path, n, &s, &error);
+        r = get_child_nodes(bus, m->path, n, true, &s, &error);
         if (r < 0)
                 return r;
         if (bus->nodes_modified)
@@ -1475,6 +1479,32 @@ void bus_node_gc(sd_bus *b, struct node *n) {
         free(n);
 }
 
+static int bus_find_parent_object_manager(sd_bus *bus, struct node **out, const char *path) {
+        struct node *n;
+
+        assert(bus);
+        assert(path);
+
+        n = hashmap_get(bus->nodes, path);
+        if (!n) {
+                char *prefix;
+
+                prefix = alloca(strlen(path) + 1);
+                OBJECT_PATH_FOREACH_PREFIX(prefix, path) {
+                        n = hashmap_get(bus->nodes, prefix);
+                        if (n)
+                                break;
+                }
+        }
+
+        while (n && !n->object_managers)
+                n = n->parent;
+
+        if (out)
+                *out = n;
+        return !!n;
+}
+
 static int bus_add_object(
                 sd_bus *bus,
                 sd_bus_slot **slot,
@@ -2277,6 +2307,7 @@ _public_ int sd_bus_emit_object_added(sd_bus *bus, const char *path) {
         BUS_DONT_DESTROY(bus);
 
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+        struct node *object_manager;
         int r;
 
         /*
@@ -2297,11 +2328,17 @@ _public_ int sd_bus_emit_object_added(sd_bus *bus, const char *path) {
         if (!BUS_IS_OPEN(bus->state))
                 return -ENOTCONN;
 
+        r = bus_find_parent_object_manager(bus, &object_manager, path);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return -ESRCH;
+
         do {
                 bus->nodes_modified = false;
                 m = sd_bus_message_unref(m);
 
-                r = sd_bus_message_new_signal(bus, &m, path, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded");
+                r = sd_bus_message_new_signal(bus, &m, object_manager->path, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded");
                 if (r < 0)
                         return r;
 
@@ -2440,6 +2477,7 @@ _public_ int sd_bus_emit_object_removed(sd_bus *bus, const char *path) {
         BUS_DONT_DESTROY(bus);
 
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+        struct node *object_manager;
         int r;
 
         /*
@@ -2460,11 +2498,17 @@ _public_ int sd_bus_emit_object_removed(sd_bus *bus, const char *path) {
         if (!BUS_IS_OPEN(bus->state))
                 return -ENOTCONN;
 
+        r = bus_find_parent_object_manager(bus, &object_manager, path);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return -ESRCH;
+
         do {
                 bus->nodes_modified = false;
                 m = sd_bus_message_unref(m);
 
-                r = sd_bus_message_new_signal(bus, &m, path, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved");
+                r = sd_bus_message_new_signal(bus, &m, object_manager->path, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved");
                 if (r < 0)
                         return r;
 
@@ -2596,6 +2640,7 @@ _public_ int sd_bus_emit_interfaces_added_strv(sd_bus *bus, const char *path, ch
         BUS_DONT_DESTROY(bus);
 
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+        struct node *object_manager;
         char **i;
         int r;
 
@@ -2609,11 +2654,17 @@ _public_ int sd_bus_emit_interfaces_added_strv(sd_bus *bus, const char *path, ch
         if (strv_isempty(interfaces))
                 return 0;
 
+        r = bus_find_parent_object_manager(bus, &object_manager, path);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return -ESRCH;
+
         do {
                 bus->nodes_modified = false;
                 m = sd_bus_message_unref(m);
 
-                r = sd_bus_message_new_signal(bus, &m, path, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded");
+                r = sd_bus_message_new_signal(bus, &m, object_manager->path, "org.freedesktop.DBus.ObjectManager", "InterfacesAdded");
                 if (r < 0)
                         return r;
 
@@ -2673,6 +2724,7 @@ _public_ int sd_bus_emit_interfaces_added(sd_bus *bus, const char *path, const c
 
 _public_ int sd_bus_emit_interfaces_removed_strv(sd_bus *bus, const char *path, char **interfaces) {
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;
+        struct node *object_manager;
         int r;
 
         assert_return(bus, -EINVAL);
@@ -2685,7 +2737,13 @@ _public_ int sd_bus_emit_interfaces_removed_strv(sd_bus *bus, const char *path,
         if (strv_isempty(interfaces))
                 return 0;
 
-        r = sd_bus_message_new_signal(bus, &m, path, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved");
+        r = bus_find_parent_object_manager(bus, &object_manager, path);
+        if (r < 0)
+                return r;
+        if (r == 0)
+                return -ESRCH;
+
+        r = sd_bus_message_new_signal(bus, &m, object_manager->path, "org.freedesktop.DBus.ObjectManager", "InterfacesRemoved");
         if (r < 0)
                 return r;
 

diff --git a/src/libsystemd/sd-bus/test-bus-objects.c b/src/libsystemd/sd-bus/test-bus-objects.c
index 1db67ec..359984c 100644 (file)
--- a/src/libsystemd/sd-bus/test-bus-objects.c
+++ b/src/libsystemd/sd-bus/test-bus-objects.c
@@ -153,7 +153,7 @@ static int notify_test2(sd_bus_message *m, void *userdata, sd_bus_error *error)
 static int emit_interfaces_added(sd_bus_message *m, void *userdata, sd_bus_error *error) {
         int r;
 
-        assert_se(sd_bus_emit_interfaces_added(sd_bus_message_get_bus(m), m->path, "org.freedesktop.systemd.test", NULL) >= 0);
+        assert_se(sd_bus_emit_interfaces_added(sd_bus_message_get_bus(m), "/value/a/x", "org.freedesktop.systemd.ValueTest", NULL) >= 0);
 
         r = sd_bus_reply_method_return(m, NULL);
         assert_se(r >= 0);
@@ -164,7 +164,7 @@ static int emit_interfaces_added(sd_bus_message *m, void *userdata, sd_bus_error
 static int emit_interfaces_removed(sd_bus_message *m, void *userdata, sd_bus_error *error) {
         int r;
 
-        assert_se(sd_bus_emit_interfaces_removed(sd_bus_message_get_bus(m), m->path, "org.freedesktop.systemd.test", NULL) >= 0);
+        assert_se(sd_bus_emit_interfaces_removed(sd_bus_message_get_bus(m), "/value/a/x", "org.freedesktop.systemd.ValueTest", NULL) >= 0);
 
         r = sd_bus_reply_method_return(m, NULL);
         assert_se(r >= 0);
@@ -175,7 +175,7 @@ static int emit_interfaces_removed(sd_bus_message *m, void *userdata, sd_bus_err
 static int emit_object_added(sd_bus_message *m, void *userdata, sd_bus_error *error) {
         int r;
 
-        assert_se(sd_bus_emit_object_added(sd_bus_message_get_bus(m), m->path) >= 0);
+        assert_se(sd_bus_emit_object_added(sd_bus_message_get_bus(m), "/value/a/x") >= 0);
 
         r = sd_bus_reply_method_return(m, NULL);
         assert_se(r >= 0);
@@ -186,7 +186,7 @@ static int emit_object_added(sd_bus_message *m, void *userdata, sd_bus_error *er
 static int emit_object_removed(sd_bus_message *m, void *userdata, sd_bus_error *error) {
         int r;
 
-        assert_se(sd_bus_emit_object_removed(sd_bus_message_get_bus(m), m->path) >= 0);
+        assert_se(sd_bus_emit_object_removed(sd_bus_message_get_bus(m), "/value/a/x") >= 0);
 
         r = sd_bus_reply_method_return(m, NULL);
         assert_se(r >= 0);
@@ -228,6 +228,14 @@ static int enumerator_callback(sd_bus *bus, const char *path, void *userdata, ch
         return 1;
 }
 
+static int enumerator2_callback(sd_bus *bus, const char *path, void *userdata, char ***nodes, sd_bus_error *error) {
+
+        if (object_path_startswith("/value/a", path))
+                assert_se(*nodes = strv_new("/value/a/x", "/value/a/y", "/value/a/z", NULL));
+
+        return 1;
+}
+
 static void *server(void *p) {
         struct context *c = p;
         sd_bus *bus = NULL;
@@ -246,7 +254,9 @@ static void *server(void *p) {
         assert_se(sd_bus_add_object_vtable(bus, NULL, "/foo", "org.freedesktop.systemd.test2", vtable, c) >= 0);
         assert_se(sd_bus_add_fallback_vtable(bus, NULL, "/value", "org.freedesktop.systemd.ValueTest", vtable2, NULL, UINT_TO_PTR(20)) >= 0);
         assert_se(sd_bus_add_node_enumerator(bus, NULL, "/value", enumerator_callback, NULL) >= 0);
+        assert_se(sd_bus_add_node_enumerator(bus, NULL, "/value/a", enumerator2_callback, NULL) >= 0);
         assert_se(sd_bus_add_object_manager(bus, NULL, "/value") >= 0);
+        assert_se(sd_bus_add_object_manager(bus, NULL, "/value/a") >= 0);
 
         assert_se(sd_bus_start(bus) >= 0);