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/libelogind/sd-bus/bus-objects.c b/src/libelogind/sd-bus/bus-objects.c
index cbdf6552f981caab1f48c3091de9623073c8459a..7e3cd86f6567a92d0912f910e462426818af9a08 100644 (file)
--- a/src/libelogind/sd-bus/bus-objects.c
+++ b/src/libelogind/sd-bus/bus-objects.c
@@ -167,6 +167,7 @@ static int add_subtree_to_set(
                 sd_bus *bus,
                 const char *prefix,
                 struct node *n,
                 sd_bus *bus,
                 const char *prefix,
                 struct node *n,

+                bool skip_subhierarchies,

                 Set *s,
                 sd_bus_error *error) {
 
                 Set *s,
                 sd_bus_error *error) {
 


@@ -198,11 +199,13 @@ static int add_subtree_to_set(
                 if (r < 0 && r != -EEXIST)
                         return r;
 
                 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;
         }
 
         return 0;


@@ -212,6 +215,7 @@ static int get_child_nodes(
                 sd_bus *bus,
                 const char *prefix,
                 struct node *n,
                 sd_bus *bus,
                 const char *prefix,
                 struct node *n,

+                bool skip_subhierarchies,

                 Set **_s,
                 sd_bus_error *error) {
 
                 Set **_s,
                 sd_bus_error *error) {
 


@@ -227,7 +231,7 @@ static int get_child_nodes(
         if (!s)
                 return -ENOMEM;
 
         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;
         if (r < 0) {
                 set_free_free(s);
                 return r;


@@ -892,7 +896,7 @@ static int process_introspect(
         assert(n);
         assert(found_object);
 
         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)
         if (r < 0)
                 return bus_maybe_reply_error(m, r, &error);
         if (bus->nodes_modified)


@@ -1158,7 +1162,7 @@ static int process_get_managed_objects(
         if (require_fallback || !n->object_managers)
                 return 0;
 
         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)
         if (r < 0)
                 return r;
         if (bus->nodes_modified)


@@ -1467,6 +1471,32 @@ void bus_node_gc(sd_bus *b, struct node *n) {
         free(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,
 static int bus_add_object(
                 sd_bus *bus,
                 sd_bus_slot **slot,


@@ -2269,6 +2299,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;
         BUS_DONT_DESTROY(bus);
 
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;

+        struct node *object_manager;

         int r;
 
         /*
         int r;
 
         /*


@@ -2289,11 +2320,17 @@ _public_ int sd_bus_emit_object_added(sd_bus *bus, const char *path) {
         if (!BUS_IS_OPEN(bus->state))
                 return -ENOTCONN;
 
         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);
 
         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;
 
                 if (r < 0)
                         return r;
 


@@ -2432,6 +2469,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;
         BUS_DONT_DESTROY(bus);
 
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;

+        struct node *object_manager;

         int r;
 
         /*
         int r;
 
         /*


@@ -2452,11 +2490,17 @@ _public_ int sd_bus_emit_object_removed(sd_bus *bus, const char *path) {
         if (!BUS_IS_OPEN(bus->state))
                 return -ENOTCONN;
 
         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);
 
         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;
 
                 if (r < 0)
                         return r;
 


@@ -2588,6 +2632,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;
         BUS_DONT_DESTROY(bus);
 
         _cleanup_bus_message_unref_ sd_bus_message *m = NULL;

+        struct node *object_manager;

         char **i;
         int r;
 
         char **i;
         int r;
 


@@ -2601,11 +2646,17 @@ _public_ int sd_bus_emit_interfaces_added_strv(sd_bus *bus, const char *path, ch
         if (strv_isempty(interfaces))
                 return 0;
 
         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);
 
         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;
 
                 if (r < 0)
                         return r;
 


@@ -2665,6 +2716,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;
 
 _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);
         int r;
 
         assert_return(bus, -EINVAL);


@@ -2677,7 +2729,13 @@ _public_ int sd_bus_emit_interfaces_removed_strv(sd_bus *bus, const char *path,
         if (strv_isempty(interfaces))
                 return 0;
 
         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;
 
         if (r < 0)
                 return r;
 

diff --git a/src/libelogind/sd-bus/test-bus-objects.c b/src/libelogind/sd-bus/test-bus-objects.c
index 1db67ecfac703420b2ec6a650edfd6e5e3ad05e6..359984c7f3045d14ccdceb29291ffad8dc88197f 100644 (file)
--- a/src/libelogind/sd-bus/test-bus-objects.c
+++ b/src/libelogind/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;
 
 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);
 
         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;
 
 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);
 
         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;
 
 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);
 
         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;
 
 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);
 
         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;
 }
 
         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;
 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_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") >= 0);

+        assert_se(sd_bus_add_object_manager(bus, NULL, "/value/a") >= 0);

 
         assert_se(sd_bus_start(bus) >= 0);
 
 
         assert_se(sd_bus_start(bus) >= 0);