Bug 24317

To clarify why this is useful for people not familiar with implementation details of Telepathy:

The Telepathy framework has a number of "connection managers", which are daemons implementing various IM protocols. They take bus names of the form "org.freedesktop.Telepathy.ConnectionManager.<name>". Similarly, Telepathy UIs take names of the form "org.freedesktop.Telepathy.Client.<name>". Mission Control is a daemon which acts somewhat like a switchboard, handing out objects on the CMs to clients, and thus needs to know which clients and CMs are running.

Currently, it has to watch all name owner changes, but it'd be better if it could watch names matching the relevant patterns.

Here's a cleaned-up implementation of this, a mere 16 months after I started it.

I made the semantics specific to bus name/interface namespaces; in effect, the rule has to match up to (and possibly including) a period in the message argument. I also narrowed the scope to matching the zeroeth argument. This makes its purpose and semantics clearer.

(It also means that in a hypothetical Kernel D-Bus future where the bus daemon is replaced by multicast Unix sockets, with clients using socket filters to filter messages based on match rules, we only need to add a header containing each period-separated prefix of the zeroth argument (if it's a string), rather than every prefix of every string argument.)

> +  dbus_bool_t namespace = FALSE;

I think is_namespace would probably be (a) clearer, and (b) safer when considering hybrid C/C++ compilers.

> Match messages whose first argument is a bus name or interface name within
> the specified namespace.

I'd prefer "whose first argument has the STRING type, and is a bus name or ...", to be completely unambiguous.

(In reply to comment #3)
> > +  dbus_bool_t namespace = FALSE;
> 
> I think is_namespace would probably be (a) clearer, and (b) safer when
> considering hybrid C/C++ compilers.
> 
> > Match messages whose first argument is a bus name or interface name within
> > the specified namespace.
> 
> I'd prefer "whose first argument has the STRING type, and is a bus name or
> ...", to be completely unambiguous.

Squashed these changes into the relevant patches.

In commit 2f7b11158b497e759:
+                                             dbus_bool_t       is_path,
+                                             dbus_bool_t       prefix);

In the implementation the latter is called is_namespace. With that change, this looks fine to me.

While sketching out client-side code using this feature, I wonder whether the option of including the namespace itself in the watch is useful.

In the current implementation, "foo.bar." matches anything below foo.bar, and "foo.bar" matches anything below it or "foo.bar" itself. I can't really think why you'd ever want the latter.

If we declared that you don't (and if you do, you can add a second rule for arg0='') then the description in the spec and the implementation might be clearer.

(I'm okay with leaving it how it is; I'm just wondering aloud.)

(In reply to comment #6)
> If we declared that you don't (and if you do, you can add a second rule for
> arg0='') then the description in the spec and the implementation might be
> clearer.

We agreed that this simplification would be useful when discussing this in person.

(In reply to comment #6)
> In the current implementation, "foo.bar." matches anything below foo.bar, and
> "foo.bar" matches anything below it or "foo.bar" itself. I can't really think
> why you'd ever want the latter.

Equally, I don't think there's any particularly compelling reason why you *wouldn't* want it to match? The worst case is you get too many signals.

The reason I'm interested in the possibility of making it match is that Bug #34870 adds path_prefix, which I think might be relying on path_prefix="/badger" matching signals from /badger as well as from /badger/mushroom, and it'd be unnecessarily confusing if arg0namespace and path_prefix didn't behave the same at boundaries.

(davidz: are you relying on that or not?)

Created attachment 45374 [details] [review]
signals.h: rename argument in declaration to match implementation

(In reply to comment #5)
> In commit 2f7b11158b497e759:
> +                                             dbus_bool_t       is_path,
> +                                             dbus_bool_t       prefix);
> 
> In the implementation the latter is called is_namespace. With that change, this
> looks fine to me.

Here, have a patch.

Created attachment 45375 [details] [review]
re-word description of arg0namespace

It's unclear at first reading whether "may contain only one element"
means "elements >= 1, as an exception to the usual rule that
elements >= 2" (which is what was intended), or "elements == 1".
 
"Like a bus name or interface name" is a little ambiguous because they
have different syntactic restrictions: specifically allow any valid bus
name, which also allows all interface names.

Created attachment 45379 [details] [review]
Document when arg0namespace was added, for completeness

Created attachment 45383 [details] [review]
Remove support for trailing "." on arg0namespace

(In reply to comment #8)
> (In reply to comment #6)
> > In the current implementation, "foo.bar." matches anything below foo.bar, and
> > "foo.bar" matches anything below it or "foo.bar" itself. I can't really think
> > why you'd ever want the latter.
> 
> Equally, I don't think there's any particularly compelling reason why you
> *wouldn't* want it to match? The worst case is you get too many signals.
> 
> The reason I'm interested in the possibility of making it match is that Bug
> #34870 adds path_prefix, which I think might be relying on
> path_prefix="/badger" matching signals from /badger as well as from
> /badger/mushroom, and it'd be unnecessarily confusing if arg0namespace and
> path_prefix didn't behave the same at boundaries.
> 
> (davidz: are you relying on that or not?)

Yeah, for the org.fd.DBus.ObjectManager, we want to match messages with the object paths

 /net/Corp/App/MushroomManager
 /net/Corp/App/MushroomManager/*

ideally with a single match rule.

Fixed in git for 1.5.0