Bug 48816

Using dbus_bus_register() would break something that currently works - dbus-send can currently send to non-bus (peer-to-peer) connections, but not to bus connections.

In dbus-monitor, the --address option does call dbus_bus_register() and dbus_bus_add_match(), which means it can be used on bus connections but not non-bus connections.

(Admittedly, bus connections are much more common.)

I think I'd prefer to deprecate --address in both cases, and add --bus=ADDRESS and --peer=ADDRESS which are unambiguous. (I'm sure there was already a bug report where I said the same, but I can't find it now.)

If you need to connect to a non-session, non-system bus with the existing dbus-send, here is a workaround:

    DBUS_SESSION_BUS_ADDRESS="ADDRESS" dbus-send --session ...

(In reply to comment #1)
> Using dbus_bus_register() would break something that currently works -
> dbus-send can currently send to non-bus (peer-to-peer) connections, but not to
> bus connections.
Sorry, I've completely forgot about non-bus connections.

> In dbus-monitor, the --address option does call dbus_bus_register() and
> dbus_bus_add_match(), which means it can be used on bus connections but not
> non-bus connections.
> 
> (Admittedly, bus connections are much more common.)
I see.

> I think I'd prefer to deprecate --address in both cases, and add --bus=ADDRESS
> and --peer=ADDRESS which are unambiguous. (I'm sure there was already a bug
> report where I said the same, but I can't find it now.)
Agree, I like --bus and --peer much better than --address.
Would you like to deprecate --address (and make it an alias for --peer) or just remove it?

> If you need to connect to a non-session, non-system bus with the existing
> dbus-send, here is a workaround:
> 
>     DBUS_SESSION_BUS_ADDRESS="ADDRESS" dbus-send --session ...
This works fine, thank you!

(In reply to comment #2)
> (I'm sure there was already a bug
> report where I said the same, but I can't find it now.)

It was a mailing list thread: http://lists.freedesktop.org/archives/dbus/2011-November/014829.html and my reply http://lists.freedesktop.org/archives/dbus/2011-December/014833.html

> Agree, I like --bus and --peer much better than --address.
> Would you like to deprecate --address (and make it an alias for --peer) or just
> remove it?

("You" here is anyone providing a revised patch, not necessarily Andrey)

Deprecate it and keep it, please. If you removed it altogether, something that currently works (dbus-send --address to a non-bus) would cease to work, which would defeat the purpose of that suggestion.

Created attachment 63602 [details] [review]
Replace --address with --peer and --bus

Comment on attachment 63602 [details] [review]
Replace --address with --peer and --bus

Review of attachment 63602 [details] [review]:
-----------------------------------------------------------------

This basically looks good to me, although I think the warning is excessive. I'll leave this open for a bit in case anyone wants to veto it, then apply it to 1.7.x if nobody does.

::: tools/dbus-send.c
@@ +278,5 @@
> +              is_bus = FALSE;
> +            }
> +	  else /* address */
> +            {
> +              fprintf (stderr, "Warning: \"--address=\" is deprecated and may be removed soon. Please, use \"--peer\" for non-bus (peer-to-peer) connections and \"--bus\" for bus connections\n");

I'd prefer this to just have a comment /* backwards-compatible */ and no warning. Realistically, we aren't going to remove this command-line option for a very long time, if at all (it only "costs" 4 lines of code, after all).

Created attachment 63799 [details] [review]
Replace --address with --peer and --bus. Review comments taken into account.

(In reply to comment #5)
> [snip]
> This basically looks good to me, although I think the warning is excessive.
> I'll leave this open for a bit in case anyone wants to veto it, then apply
> it to 1.7.x if nobody does.
> [snip]

Could you, please, finally apply the patch?
The bug has been open for more than a year.
Or are there any other reason we should wait more?

(In reply to comment #7)
> Could you, please, finally apply the patch?

Sorry, this bug wasn't keyworded 'patch' so I kept missing it when looking for pending patches.

Your patch conflicted with changes that had been made on master, but I've adjusted and applied it. Fixed in git for 1.7.6.

(In reply to comment #8)
> (In reply to comment #7)
> > Could you, please, finally apply the patch?
> 
> Sorry, this bug wasn't keyworded 'patch' so I kept missing it when looking
> for pending patches.
> 
> Your patch conflicted with changes that had been made on master, but I've
> adjusted and applied it. Fixed in git for 1.7.6.

Thank you very much!