After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 686920 - gdbus: Allow GDBusObjectManagerClient to work on peer connections
gdbus: Allow GDBusObjectManagerClient to work on peer connections
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gdbus
unspecified
Other All
: Normal normal
: ---
Assigned To: David Zeuthen (not reading bugmail)
gtkdev
Depends on:
Blocks:
 
 
Reported: 2012-10-26 08:34 UTC by Stef Walter
Modified: 2012-10-26 19:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gdbus: Allow GDBusObjectManagerClient to work on peer connections (19.38 KB, patch)
2012-10-26 08:37 UTC, Stef Walter
reviewed Details | Review
gdbus: Allow GDBusObjectManagerClient to work on peer connections (19.38 KB, patch)
2012-10-26 16:03 UTC, Stef Walter
committed Details | Review

Description Stef Walter 2012-10-26 08:34:00 UTC
I was trying to use GDBusObjectManager on a peer connection (a fallback
when no bus daemon is available, such as in a chroot in an installer).

Anyway, when you try this you get an assertion about the bus name
being NULL. Attached patch fixes the issue.
Comment 1 Stef Walter 2012-10-26 08:37:11 UTC
Created attachment 227333 [details] [review]
gdbus: Allow GDBusObjectManagerClient to work on peer connections

Allow GDBusObjectManagerClient to work on peer to peer DBus
connections. Don't require that a unique bus name is available
for the object manager, if the owned bus name is NULL.
Comment 2 David Zeuthen (not reading bugmail) 2012-10-26 13:50:44 UTC
(Note that bug 662718 is somewhat related.)
Comment 3 David Zeuthen (not reading bugmail) 2012-10-26 14:12:04 UTC
Review of attachment 227333 [details] [review]:

Looks very solid - about adding a completely new test (gio/tests/gdbus-peer-object-manager.c), that's fine too but it would also work just adding code to the gio/tests/gdbus-test-codegen.c file. I don't care either way.

Also, instead of defining your own MockInterface type in C (and suffer from it, cf. the "Groan" comment :-) ..), I think it would be better to just add that interface to gio/tests/test-codegen.xml and then include gio/tests/gdbus-test-codegen-generated.[ch] in your code. Thoughts?

::: gio/gdbusobjectmanagerclient.c
@@ +1109,1 @@
+  if (name_owner)

The coding style in this file is generally to use "name_owner != NULL" </nitpick>
Comment 4 Stef Walter 2012-10-26 16:03:21 UTC
Created attachment 227370 [details] [review]
gdbus: Allow GDBusObjectManagerClient to work on peer connections

Allow GDBusObjectManagerClient to work on peer to peer DBus
connections. Don't require that a unique bus name is available
for the object manager, if the owned bus name is NULL.
Comment 5 Stef Walter 2012-10-26 16:07:07 UTC
(In reply to comment #3)
> Review of attachment 227333 [details] [review]:
> 
> Looks very solid - about adding a completely new test
> (gio/tests/gdbus-peer-object-manager.c), that's fine too but it would also work
> just adding code to the gio/tests/gdbus-test-codegen.c file. I don't care
> either way.

See below.

> Also, instead of defining your own MockInterface type in C (and suffer from it,
> cf. the "Groan" comment :-) ..), I think it would be better to just add that
> interface to gio/tests/test-codegen.xml and then include
> gio/tests/gdbus-test-codegen-generated.[ch] in your code. Thoughts?

I'm thinking that I'm slowly learning enough and getting up enough courage to make implementing dbus interfaces in gdbus more straight forward, via mixins, with less boilerplate. Obviously the codegen will always have its place.

But at the end of the day, I believe that a test should have as little dependent code as possible. And if we're not testing the codegen here, then I'd default to not using it if possible.

That's also why I put it in its own file, so it's not mixed in with the other codegen stuff, and it's more clear what is being tested, and what might be broken when the test fails.

But if you insist, I can change it over to use the codegen and merge it with the other tests. LMK.

> ::: gio/gdbusobjectmanagerclient.c
> @@ +1109,1 @@
> +  if (name_owner)
> 
> The coding style in this file is generally to use "name_owner != NULL"
> </nitpick>

Fixed.
Comment 6 David Zeuthen (not reading bugmail) 2012-10-26 17:05:33 UTC
Comment on attachment 227370 [details] [review]
gdbus: Allow GDBusObjectManagerClient to work on peer connections

OK, sounds good to me.
Comment 7 Stef Walter 2012-10-26 19:22:16 UTC
Attachment 227370 [details] pushed as fb2d3aa - gdbus: Allow GDBusObjectManagerClient to work on peer connections