Bustle should add "eavesdrop=true" to its rules, according to Bug #37890
Created attachment 49009 [details] [review]
Add eavesdrop=true to bustle-dbus-monitor.c
Same fix dbus-monitor.c received in Bug #37890 fix.
Just found a bug in the patch, I'll submit a new one ASAP
Created attachment 49012 [details] [review]
add eavesdrop=true when supported
It checks for support adding the rule once, and then selects what to use depending on AddMatch error status.
From the prev. patch, it fixes the case with no filter passed at command line, adding the rule also under this case.
Created attachment 49013 [details] [review]
Since I was working on it and I fixed the same problem in dbus-monitor.
Created attachment 49014 [details] [review]
filters needs to be freed
It also seems that filters need to be freed as well.
Review of attachment 49012 [details] [review]:
@@ +369,3 @@
+ /* check support for eavesdrop=true in filter */
+ dbus_bus_add_match (connection, EAVESDROPPING_RULE, &error);
This is an interesting way to check for support, but it has the flaw that bustle will receive any messages between this temporary match being added, and the same match being removed, even if they shouldn't have matched.
One way to make this not a practical problem would be to use "eavesdrop='true',destination=':1.23'" (replacing :1.23 with our own unique name), which only matches messages that we ought to be receiving anyway.
However, I'd personally do this:
- provide a function bustle_add_match() and use it to replace
all calls to dbus_bus_add_match()
- in that function:
- have a static variable eavesdrop_support taking values
EAVESDROP_SUPPORTED, EAVESDROP_UNSUPPORTED, EAVESDROP_UNKNOWN
- first try prepending "eavesdrop='true'," and using that,
unless we already know it's not supported
- if that succeeds, set EAVESDROP_SUPPORTED
- if that fails with MatchRuleInvalid and EAVESDROP_SUPPORTED
is not set, try again without the prefix; if that works,
- on any other failure, just fail
@@ +375,3 @@
+ /* make it re-usable */
+ dbus_error_free (&error);
+ dbus_error_init (&error);
dbus_error_free is enough: its documentation says that it "reinitializes the error as in dbus_error_init".
@@ +377,3 @@
+ dbus_error_init (&error);
+ /* using a dbus version witout this feature, too bad, but still OK */
@@ +392,3 @@
+ if (dbus_error_is_set (&error))
+ fprintf (stderr, "Failed to removed rule \"%s\": %s\n",
"Failed to remove rule"
Review of attachment 49013 [details] [review]:
Looks good to me (although I'm not wjt)
Review of attachment 49014 [details] [review]:
Looks fine, although leaking O(1) memory isn't really a problem.
Created attachment 49396 [details] [review]
Created attachment 49397 [details] [review]
Add eavesdrop=true when supported
Use bustle_add_match() as Simon suggested
Created attachment 49398 [details] [review]
Free filters too
useless but since has been spotted... :)
Merged to master:
And I added an equivalent fix to the new pcap-based logger, which is the standard one henceforth:
I haven't got dbus 1.5.x on this system; Cosimo, could you cast your eye over it to see if you think it's right? I'm pretty sure it should work, from reading the logs. The fallback certainly works. :)
Anyway, these fixes will be included in Bustle 0.3.2. Thanks!
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct.