Bug 26548

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.

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>

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"

Created attachment 45571 [details] [review]
monitor: bail if asked to monitor >1 bus

Fixes: <https://bugs.freedesktop.org/show_bug.cgi?id=26548>

Review of attachment 45571 [details] [review]:

Looks good, thanks.

Merged for 1.4.10, 1.5.2