Re: [systemd-devel] [RFC] bus: add sd_bus_emit_object_{added, removed}()
Hi
On Tue, Feb 18, 2014 at 12:02 AM, David Herrmann dh.herrm...@gmail.com wrote:
The ObjectManager dbus interface provides an InterfacesAdded signal to
notify others about new interfaces that are added to an object. The same
signal is also used to advertise new objects (by adding the first
interface to a given object path) and delete them.
However, our internal helpers sd_bus_emit_interfaces_{added,removed}()
cannot properly deal with built-in interfaces like DBus.Properties as
there's no vtable for it. Therefore, to avoid callers to track these
internal interfaces, we provide two separate helpers which explicitly add
these interfaces to the signal.
sd_bus_emit_object_added() traverses the list of all vtables and fallback
vtables (making sure a fallback never overwrites a real vtable!) and also
adds built-in interfaces.
sd_bus_emit_object_removed(): WIP
---
Hi
This is untested and I just wanted to get some feedback whether that's the way
to go. Given the previous discussion we decided on two new entry-points to add
and remove objects. For convenience, I now tried to omit any char
**interfaces
argument to object_added() and just try to figure them out on my own.
However, on object_removed() I cannot do that as node_vtable_get_userdata() is
very likely to return 0 for all these objects. So there is no way to figure
out which interfaces actually existed on that thing. We would require users to
call it *before* destroying/unlinking the actual object. I don't know whether
that's ok to assume?
If not, we can just add a char **interfaces argument, but then it would
differ
from object_added().. Not sure what sounds better..
Cheers
David
src/libsystemd/sd-bus/bus-objects.c | 210
src/systemd/sd-bus.h| 2 +
2 files changed, 212 insertions(+)
After almost 1 year of niche existence I finally went ahead and
re-implemented this. Review is welcome!
Pushed.
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] bus: add sd_bus_emit_object_{added, removed}()
On Tue, 18.02.14 00:02, David Herrmann (dh.herrm...@gmail.com) wrote:
Sorry for the late review!
However, on object_removed() I cannot do that as node_vtable_get_userdata() is
very likely to return 0 for all these objects. So there is no way to figure
out which interfaces actually existed on that thing. We would require users to
call it *before* destroying/unlinking the actual object. I don't know whether
that's ok to assume?
I would assume that it is OK to assume that. People should call this
function before half-destroying their object. I mean we are not reading
the properties after all, just the interfaces and I think that should be
quite OK to require.
Certainly something to document though one day...
+static int object_added_append_all_prefix(
+sd_bus *bus,
+sd_bus_message *m,
+Set *s,
+const char *prefix,
+const char *path,
+bool require_fallback) {
+
+if (!streq_ptr(c-interface, previous_interface)) {
+/* interface already handled by a previous run? */
+if (set_get(s, c-interface))
+continue;
We actually allow multiple vtables with the same interface, and order
them together in the vtable list, so that we can iterate through them
easily with trivial duplicate reduction. Keeping a Set object here
appears unnecessary? Or did I miss something here?
Otherwise looks good!
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] bus: add sd_bus_emit_object_{added, removed}()
Hi
On Tue, Mar 11, 2014 at 7:27 PM, Lennart Poettering
lenn...@poettering.net wrote:
On Tue, 18.02.14 00:02, David Herrmann (dh.herrm...@gmail.com) wrote:
Sorry for the late review!
However, on object_removed() I cannot do that as node_vtable_get_userdata()
is
very likely to return 0 for all these objects. So there is no way to figure
out which interfaces actually existed on that thing. We would require users
to
call it *before* destroying/unlinking the actual object. I don't know whether
that's ok to assume?
I would assume that it is OK to assume that. People should call this
function before half-destroying their object. I mean we are not reading
the properties after all, just the interfaces and I think that should be
quite OK to require.
Certainly something to document though one day...
Yepp, will add docs once I commit it.
+static int object_added_append_all_prefix(
+sd_bus *bus,
+sd_bus_message *m,
+Set *s,
+const char *prefix,
+const char *path,
+bool require_fallback) {
+
+if (!streq_ptr(c-interface, previous_interface)) {
+/* interface already handled by a previous run? */
+if (set_get(s, c-interface))
+continue;
We actually allow multiple vtables with the same interface, and order
them together in the vtable list, so that we can iterate through them
easily with trivial duplicate reduction. Keeping a Set object here
appears unnecessary? Or did I miss something here?
A single call to object_added_append_all_prefix() doesn't need Set
*s as it's ordered (as you said). The problem is more subtle, see
object_added_append_all(), where I do this:
+r = object_added_append_all_prefix(bus, m, s, path, path, false);
+if (r 0)
+return r;
+if (bus-nodes_modified)
+return 0;
+
+prefix = alloca(strlen(path) + 1);
+OBJECT_PATH_FOREACH_PREFIX(prefix, path) {
+r = object_added_append_all_prefix(bus, m, s, prefix,
path, true);
+if (r 0)
+return r;
+if (bus-nodes_modified)
+return 0;
+}
First I call append_all_prefix() with the exact path, which cannot
ever fire the if (set_get()) protection, as it's the first run.
However, the prefix-runs which I do afterwards will traverse the
parents, which might *also* implement the interface. They use
require_fallback == true, so usually we're fine. However, if you
implement an interface explicitly on a child, but as fallback on the
parents, I _must_ avoid adding the interface twice. At least that's my
understanding of the vtable API, or should these be merged?
I looked at interfaces_added_append_one(), which skips parents if the
node already implements the interface. Therefore, I assumed I have to
skip parents, too. But I still need to traverse the parents, because
they might implement different interfaces that I haven't found, yet.
Thanks
David
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [RFC] bus: add sd_bus_emit_object_{added, removed}()
ping?
On Tue, Feb 18, 2014 at 12:02 AM, David Herrmann dh.herrm...@gmail.com wrote:
The ObjectManager dbus interface provides an InterfacesAdded signal to
notify others about new interfaces that are added to an object. The same
signal is also used to advertise new objects (by adding the first
interface to a given object path) and delete them.
However, our internal helpers sd_bus_emit_interfaces_{added,removed}()
cannot properly deal with built-in interfaces like DBus.Properties as
there's no vtable for it. Therefore, to avoid callers to track these
internal interfaces, we provide two separate helpers which explicitly add
these interfaces to the signal.
sd_bus_emit_object_added() traverses the list of all vtables and fallback
vtables (making sure a fallback never overwrites a real vtable!) and also
adds built-in interfaces.
sd_bus_emit_object_removed(): WIP
---
Hi
This is untested and I just wanted to get some feedback whether that's the way
to go. Given the previous discussion we decided on two new entry-points to add
and remove objects. For convenience, I now tried to omit any char
**interfaces
argument to object_added() and just try to figure them out on my own.
However, on object_removed() I cannot do that as node_vtable_get_userdata() is
very likely to return 0 for all these objects. So there is no way to figure
out which interfaces actually existed on that thing. We would require users to
call it *before* destroying/unlinking the actual object. I don't know whether
that's ok to assume?
If not, we can just add a char **interfaces argument, but then it would
differ
from object_added().. Not sure what sounds better..
Cheers
David
src/libsystemd/sd-bus/bus-objects.c | 210
src/systemd/sd-bus.h| 2 +
2 files changed, 212 insertions(+)
diff --git a/src/libsystemd/sd-bus/bus-objects.c
b/src/libsystemd/sd-bus/bus-objects.c
index b116a5d..0c099a3 100644
--- a/src/libsystemd/sd-bus/bus-objects.c
+++ b/src/libsystemd/sd-bus/bus-objects.c
@@ -2469,6 +2469,216 @@ _public_ int sd_bus_emit_interfaces_removed(sd_bus
*bus, const char *path, const
return sd_bus_emit_interfaces_removed_strv(bus, path, interfaces);
}
+static int object_added_append_all_prefix(
+sd_bus *bus,
+sd_bus_message *m,
+Set *s,
+const char *prefix,
+const char *path,
+bool require_fallback) {
+
+_cleanup_bus_error_free_ sd_bus_error error = SD_BUS_ERROR_NULL;
+const char *previous_interface = NULL;
+struct node_vtable *c;
+struct node *n;
+void *u = NULL;
+int r;
+
+assert(bus);
+assert(m);
+assert(s);
+assert(prefix);
+assert(path);
+
+n = hashmap_get(bus-nodes, prefix);
+if (!n)
+return 0;
+
+LIST_FOREACH(vtables, c, n-vtables) {
+if (require_fallback !c-is_fallback)
+continue;
+
+r = node_vtable_get_userdata(bus, path, c, u, error);
+if (r 0)
+return r;
+if (bus-nodes_modified)
+return 0;
+if (r == 0)
+continue;
+
+if (!streq_ptr(c-interface, previous_interface)) {
+/* interface already handled by a previous run? */
+if (set_get(s, c-interface))
+continue;
+
+/* prevent fallbacks from overwriting specific objs
*/
+r = set_put(s, c-interface);
+if (r 0)
+return r;
+
+if (previous_interface) {
+r = sd_bus_message_close_container(m);
+if (r 0)
+return r;
+
+r = sd_bus_message_close_container(m);
+if (r 0)
+return r;
+}
+
+r = sd_bus_message_open_container(m, 'e', sa{sv});
+if (r 0)
+return r;
+
+r = sd_bus_message_append_basic(m, 's',
c-interface);
+if (r 0)
+return r;
+
+r = sd_bus_message_open_container(m, 'a', {sv});
+if (r 0)
+return r;
+
+previous_interface = c-interface;
+}
+
+r = vtable_append_all_properties(bus, m, path, c, u, error);
+if (r 0)
+return r;
+
