Bug 39140 - add eavesdrop=true to rule(s)
add eavesdrop=true to rule(s)
Product: Bustle
Classification: Unclassified
Component: General
Other All
: medium normal
Assigned To: Cosimo Alfarano
Depends on: 37890
  Show dependency treegraph
Reported: 2011-07-11 07:47 UTC by Cosimo Alfarano
Modified: 2012-01-13 03:41 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

Add eavesdrop=true to bustle-dbus-monitor.c (2.36 KB, patch)
2011-07-12 07:38 UTC, Cosimo Alfarano
Details | Splinter Review
add eavesdrop=true when supported (4.96 KB, patch)
2011-07-12 09:54 UTC, Cosimo Alfarano
Details | Splinter Review
handle OOM (1.21 KB, patch)
2011-07-12 09:55 UTC, Cosimo Alfarano
Details | Splinter Review
filters needs to be freed (633 bytes, patch)
2011-07-12 09:56 UTC, Cosimo Alfarano
Details | Splinter Review
OOM (1.21 KB, patch)
2011-07-21 10:05 UTC, Cosimo Alfarano
Details | Splinter Review
Add eavesdrop=true when supported (4.97 KB, patch)
2011-07-21 10:06 UTC, Cosimo Alfarano
Details | Splinter Review
Free filters too (693 bytes, patch)
2011-07-21 10:07 UTC, Cosimo Alfarano
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Cosimo Alfarano 2011-07-11 07:47:11 UTC
Bustle should add "eavesdrop=true" to its rules, according to Bug #37890
Comment 1 Cosimo Alfarano 2011-07-12 07:38:24 UTC
Created attachment 49009 [details] [review]
Add eavesdrop=true to bustle-dbus-monitor.c

Same fix dbus-monitor.c received in Bug #37890 fix.
Comment 2 Cosimo Alfarano 2011-07-12 07:52:47 UTC
Just found a bug in the patch, I'll submit a new one ASAP
Comment 3 Cosimo Alfarano 2011-07-12 09:54:15 UTC
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.
Comment 4 Cosimo Alfarano 2011-07-12 09:55:07 UTC
Created attachment 49013 [details] [review]
handle OOM

Since I was working on it and I fixed the same problem in dbus-monitor.
Comment 5 Cosimo Alfarano 2011-07-12 09:56:08 UTC
Created attachment 49014 [details] [review]
filters needs to be freed

It also seems that filters need to be freed as well.
Comment 6 Simon McVittie 2011-07-19 07:11:20 UTC
Review of attachment 49012 [details] [review]:

::: bustle-dbus-monitor.c
@@ +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
    or something

  - 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"
Comment 7 Simon McVittie 2011-07-19 07:12:20 UTC
Review of attachment 49013 [details] [review]:

Looks good to me (although I'm not wjt)
Comment 8 Simon McVittie 2011-07-19 07:15:21 UTC
Review of attachment 49014 [details] [review]:

Looks fine, although leaking O(1) memory isn't really a problem.
Comment 9 Cosimo Alfarano 2011-07-21 10:05:23 UTC
Created attachment 49396 [details] [review]
Comment 10 Cosimo Alfarano 2011-07-21 10:06:41 UTC
Created attachment 49397 [details] [review]
Add eavesdrop=true when supported

Use bustle_add_match() as Simon suggested
Comment 11 Cosimo Alfarano 2011-07-21 10:07:34 UTC
Created attachment 49398 [details] [review]
Free filters too

useless but since has been spotted... :)
Comment 12 Will Thompson 2012-01-13 03:41:26 UTC
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!