Bug 15412 - [patch] dbus-send needs support for peer-to-peer private bus
Summary: [patch] dbus-send needs support for peer-to-peer private bus
Status: RESOLVED FIXED
Alias: None
Product: dbus
Classification: Unclassified
Component: core (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: Havoc Pennington
QA Contact: John (J5) Palmieri
URL:
Whiteboard:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2008-04-08 19:39 UTC by Lawrence R. Steeger
Modified: 2008-10-18 11:51 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
Enable private peer-to-peer message bus access/fix man page (5.10 KB, patch)
2008-04-08 19:39 UTC, Lawrence R. Steeger
Details | Splinter Review
Enable private peer-to-peer message bus access/fix man page (5.09 KB, patch)
2008-04-09 04:51 UTC, Lawrence R. Steeger
Details | Splinter Review
Add peer-to-peer message bus access/fix man page/fix coding style (20.07 KB, patch)
2008-04-09 07:03 UTC, Lawrence R. Steeger
Details | Splinter Review
Add peer-to-peer message bus access/fix man page/fix coding style (5.12 KB, patch)
2008-04-10 05:10 UTC, Lawrence R. Steeger
Details | Splinter Review
Add --address=ADDRESS option (2.61 KB, patch)
2008-04-10 12:01 UTC, Lawrence R. Steeger
Details | Splinter Review
ChangeLog entry for --address=ADDRESS patch (196 bytes, text/plain)
2008-04-10 12:02 UTC, Lawrence R. Steeger
Details
Add expanded error messages for options and arguments (1.00 KB, text/plain)
2008-04-10 12:04 UTC, Lawrence R. Steeger
Details
ChangeLog entry for expanded error messages (292 bytes, text/plain)
2008-04-10 12:04 UTC, Lawrence R. Steeger
Details
Man page updates for --address=ADDRESS, etal. (2.28 KB, patch)
2008-04-10 12:06 UTC, Lawrence R. Steeger
Details | Splinter Review
ChangLog entry for man page updates (273 bytes, text/plain)
2008-04-10 12:06 UTC, Lawrence R. Steeger
Details
Add expanded error messages for options and arguments (1.00 KB, patch)
2008-04-10 12:09 UTC, Lawrence R. Steeger
Details | Splinter Review
Add --address=ADDRESS option (2.67 KB, patch)
2008-04-10 12:23 UTC, Lawrence R. Steeger
Details | Splinter Review
Add --address=ADDRESS option (2.67 KB, patch)
2008-04-10 12:48 UTC, Lawrence R. Steeger
Details | Splinter Review

Description Lawrence R. Steeger 2008-04-08 19:39:13 UTC
Created attachment 15772 [details] [review]
Enable private peer-to-peer message bus access/fix man page

The HAL 0.5.10 Specification at http://people.freedesktop.org/~david/hal-spec/hal-spec.html#device-properties-info-callouts
says:

"...callouts can (and should) communicate with the HAL daemon using a peer to peer D-Bus connection specified by the HALD_DIRECT_ADDR environment variable. ..."

To enable this in a Bash script requires a modified dbus-send command with support for using this environment variable's content.

I've attached a patch that adds this functionality.

Also, Bug 14005, "dbus-send manpage and dbus --help differ" is fixed in this patch
because I needed to update the man page anyway.


Files modified:

tools/dbus-send.c
tools/dbus-send.1
Comment 1 Lawrence R. Steeger 2008-04-09 04:51:54 UTC
Created attachment 15781 [details] [review]
Enable private peer-to-peer message bus access/fix man page

Fix some text in dbus-send.c in prior patch.
No logic changes.
Comment 2 Timo Hoenig 2008-04-09 05:51:21 UTC
The second version is not much better.

Please
* do not use tabs.
* remove trailing whitespace.
* add a new line before '{'
...

Just compare your code to the rest of dbus-send.c, it's obvious.
Comment 3 Lawrence R. Steeger 2008-04-09 07:03:45 UTC
Created attachment 15782 [details] [review]
Add peer-to-peer message bus access/fix man page/fix coding style

Fixed coding style.
Comment 4 Timo Hoenig 2008-04-10 00:57:28 UTC
Sorry, Lawrence, but it seems that the new patch does not match the requirements either.  You shall not run indent on the patched file.

Just add the bits you want to get reviewed and stick coding style of the existing code.

Your latest patch changes the whole file which doesn't help us on reviewing your changes.

Please understand that we have to stick to those rules to keep the project's source code in a consistent shape.  Thanks you.
Comment 5 Lawrence R. Steeger 2008-04-10 05:10:52 UTC
Created attachment 15801 [details] [review]
Add peer-to-peer message bus access/fix man page/fix coding style

Hopefully, last coding style request.

Please note that the dbus-send.c source from git://anongit.freedesktop.org/git/dbus/dbus contains several coding style errors of its own (i.e., tabs and indentation).
Comment 6 Timo Hoenig 2008-04-10 05:24:57 UTC
This one looks much better, thank you.

Havoc, J5: Could you please have a look at the last revision of the patch?  Thanks!
Comment 7 John (J5) Palmieri 2008-04-10 08:32:14 UTC
Thank you for bearing with us.  D-Bus is on it's way to becoming an actual standard (it is defacto right now) so we are a little stricter with the code than most other projects.  Technically the patch looks good but there are a couple of things that need to be fixed before we can commit:

There are a couple of patches in there.  You don't have to open another bug but can you split out the expanded error messages that do not relate to the peerbus with the core of your patch?  I want to commit them separately with separate ChangeLog entries.   Also please attach detailed ChangeLogs (look at the ChangeLog file for format).

Peerbus and private aren't descriptive enough.  You can open any address even the session bus and system bus with connection_open, as you only have one connection per invocation of dbus_send, public and private connections have no real meaning.  A better moniker is simply --address and address as the variable (or connection_address).

It might be there but I didn't see it.  Can you print out an error if --address is specified and either --session or --system is also specified?

And the last point.  This patch is significant enough for you to get your name in the AUTHORS file.  We also need you to make a statement in this bug that you agree to license this patch under the MIT Permissive license.  Though we ship GPL/Artistic all new code must be explicitly licensed under the MIT Permissive license as we are trying to move to that license to eliminate confusion which crops up with our current licensing scheme.  Thanks.

Comment 8 Lawrence R. Steeger 2008-04-10 12:01:39 UTC
Created attachment 15813 [details] [review]
Add --address=ADDRESS option

The --address=ADDRESS option allows direct connection to a specified address.

It is mutually exclusive with --session and --system.

Patch contents licensed under the MIT Permissive license.
Comment 9 Lawrence R. Steeger 2008-04-10 12:02:39 UTC
Created attachment 15814 [details]
ChangeLog entry for --address=ADDRESS patch
Comment 10 Lawrence R. Steeger 2008-04-10 12:04:08 UTC
Created attachment 15815 [details]
Add expanded error messages for options and arguments

See ChangeLog attachement for details.

Patch contents licensed under the MIT Permissive license.
Comment 11 Lawrence R. Steeger 2008-04-10 12:04:51 UTC
Created attachment 15816 [details]
ChangeLog entry for expanded error messages

Patch contents licensed under the MIT Permissive license.
Comment 12 Lawrence R. Steeger 2008-04-10 12:06:00 UTC
Created attachment 15818 [details] [review]
Man page updates for --address=ADDRESS, etal.

See ChangeLog entry for details.

Patch contents licensed under the MIT Permissive license.
Comment 13 Lawrence R. Steeger 2008-04-10 12:06:42 UTC
Created attachment 15819 [details]
ChangLog entry for man page updates

Patch contents licensed under the MIT Permissive license.
Comment 14 Lawrence R. Steeger 2008-04-10 12:09:19 UTC
Created attachment 15820 [details] [review]
Add expanded error messages for options and arguments

See ChangeLog entry for details.

Patch contents licensed under the MIT Permissive license.
Comment 15 Lawrence R. Steeger 2008-04-10 12:23:01 UTC
Created attachment 15822 [details] [review]
Add --address=ADDRESS option

Add --address=ADDRESS option

The --address=ADDRESS option allows direct connection to a specified address.

It is mutually exclusive with --session and --system.

Patch contents licensed under the MIT Permissive license.

(small problem in prior patch. lrs)
Comment 16 Lawrence R. Steeger 2008-04-10 12:48:02 UTC
Created attachment 15823 [details] [review]
Add --address=ADDRESS option

Add --address=ADDRESS option

The --address=ADDRESS option allows direct connection to a specified address.

It is mutually exclusive with --session and --system.

Patch contents licensed under the MIT Permissive license.

(small problem in prior patch. lrs)
Comment 17 Lawrence R. Steeger 2008-04-10 12:50:06 UTC
Ok, all done and ready for comments.

Thanks.
Comment 18 Colin Walters 2008-10-18 11:51:41 UTC
Applied - sorry about the delay.

commit 14afa0564e9eea01d28d4b2fd1e6ac0bfec626e7
Author: Lawrence R. Steeger <lsteeger@gmail.com>
Date:   Sat Oct 18 14:50:49 2008 -0400

    Bug 15412: Add --address option to dbus-send
    Signed-off-by: Colin Walters <walters@verbum.org>



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.