[systemd-devel] [RFC] bus: add sd_bus_emit_object_{added, removed}()

[David Herrmann]

Hi

On Tue, Mar 11, 2014 at 7:27 PM, Lennart Poettering
<lennart at poettering.net> wrote:
> On Tue, 18.02.14 00:02, David Herrmann (dh.herrmann at 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