From: David Herrmann Date: Tue, 21 Jul 2015 10:59:56 +0000 (+0200) Subject: sd-bus: fix path of object-manager signals X-Git-Tag: v226.4~1^2~194 X-Git-Url: https://www.chiark.greenend.org.uk/ucgi/~ianmdlvl/git?p=elogind.git;a=commitdiff_plain;h=31b940441e3c62fcdba8943307955df357cce085;hp=6609ed3a939c2feb8a7cd39ce4f45413f2d58315 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 cbdf6552f..7e3cd86f6 100644 --- 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, + bool skip_subhierarchies, Set *s, sd_bus_error *error) { @@ -198,11 +199,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; @@ -212,6 +215,7 @@ static int get_child_nodes( sd_bus *bus, const char *prefix, struct node *n, + bool skip_subhierarchies, Set **_s, sd_bus_error *error) { @@ -227,7 +231,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; @@ -892,7 +896,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) @@ -1158,7 +1162,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) @@ -1467,6 +1471,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, @@ -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; + struct node *object_manager; 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; + 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; @@ -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; + struct node *object_manager; 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; + 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; @@ -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; + struct node *object_manager; 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; + 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; @@ -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; + struct node *object_manager; 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; - 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/libelogind/sd-bus/test-bus-objects.c b/src/libelogind/sd-bus/test-bus-objects.c index 1db67ecfa..359984c7f 100644 --- 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; - 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);