Bug 26548 - Make dbus-monitor complain if asked to monitor more than one bus
Summary: Make dbus-monitor complain if asked to monitor more than one bus
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Will Thompson
QA Contact: John (J5) Palmieri
URL: http://git.collabora.co.uk/?p=user/wj...
Whiteboard: review+ walters/smcv
Keywords: patch
Depends on:
Blocks: dbus-1.5
  Show dependency treegraph
 
Reported: 2010-02-12 11:23 UTC by Will Thompson
Modified: 2011-04-29 07:49 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
minotaur: bail if asked to monitor >1 bus (2.33 KB, patch)
2011-04-12 08:54 UTC, Will Thompson
Details | Splinter Review
monitor: bail if asked to monitor >1 bus (2.17 KB, patch)
2011-04-13 03:58 UTC, Will Thompson
Details | Splinter Review

Description Will Thompson 2010-02-12 11:23:42 UTC
Currently it silently accepts both --session and --system, and just picks the last one. This tripped someone here up today!
Comment 1 Simon McVittie 2011-01-21 04:54:04 UTC
Good in principle, but:

> +static void
> +only_one_type (dbus_bool_t *seen_bus_type,
> +               char        *name)
> +{
> +  if (*seen_bus_type)
> +    {
> +      fprintf (stderr, "I only support monitoring one bus at a time!\n");
> +      usage (name, 1);
> +    }

@name here ought to be "dbus-monitor" or "/usr/bin/dbus-monitor" or something...

>        if (!strcmp (arg, "--system"))
> -       type = DBUS_BUS_SYSTEM;
> +        {
> +          only_one_type (&seen_bus_type, arg);

... so it should be argv[0] here.

With the patch as-is, you'll print

    Usage: --session [--blah|--blah|--blah]

instead of the intended

    Usage: dbus-monitor [--blah|--blah|--blah]

Apart from that it looks fine, though.
Comment 2 Will Thompson 2011-04-12 08:54:21 UTC
Created attachment 45536 [details] [review]
minotaur: bail if asked to monitor >1 bus

A coworker was just tripped up by `dbus-monitor --session --system` only
monitoring the system bus. This patch would have saved him reproducing a
tricky bug several times!

Fixes: <https://bugs.freedesktop.org/show_bug.cgi?id=26548>
Comment 3 Colin Walters 2011-04-12 13:35:03 UTC
Review of attachment 45536 [details] [review]:

::: tools/dbus-monitor.c
@@ +222,3 @@
+  if (*seen_bus_type)
+    {
+      fprintf (stderr, "I only support monitoring one bus at a time!\n");

I think we should avoid first person colloquial for error messages.  How about:

"error: Multiple bus or address specifications given"
Comment 4 Will Thompson 2011-04-13 03:58:23 UTC
Created attachment 45571 [details] [review]
monitor: bail if asked to monitor >1 bus

Fixes: <https://bugs.freedesktop.org/show_bug.cgi?id=26548>
Comment 5 Colin Walters 2011-04-13 07:31:12 UTC
Review of attachment 45571 [details] [review]:

Looks good, thanks.
Comment 6 Simon McVittie 2011-04-29 07:49:51 UTC
Merged for 1.4.10, 1.5.2


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.