Created attachment 192925 [details] [review]
patch

Christian, what i'm concerned about is that this change will break the experience for people who have an older gconf daemon and then use client using a newer version of the library at it.

We could mitigate this by checking the signature on the message.

I guess you're concerned about gconfd on devices which shipped this dbus-enabled gconfd long before it was merged upstream, right?

I think if this is the case, we could just keep using "s" here but add a check that the string actually is an object path.

(I ran into this while porting libgconf to gdbus again, and just wanted to raise the issue.)

I was thinking of the desktop case with some components built from jhbuild. e.g. old gconfd with newer client. One simple way to give some transition is to switch the server to return an object path and then in the client code check the signature.

Now, since this change would be API change, do you think we should try and get it into GNOME 3.1.5 - if so need to spin a 3.1.5.1 given I did 3.1.5 yesterday ;-)

Landed!

commit 38f1973d58c65639fe83de7918b5c765933302cd
Author: Rob Bradford <rob@linux.intel.com>
Date:   Mon Aug 15 15:24:27 2011 +0100

    gconf-dbus: Support older daemon returning string for object path
    
    Enhances: https://bugzilla.gnome.org/show_bug.cgi?id=655656

commit 6736f42ce5172b44083a9089ff4a777ed1c5bdc3
Author: Christian Persch <chpe@gnome.org>
Date:   Sat Jul 30 23:06:53 2011 +0200

    gconf(d)-dbus: Use correct type, 'o', for object paths
    
    Fixes: https://bugzilla.gnome.org/show_bug.cgi?id=655656

Thanks! I discovered that there's another place that needs the same treatment, the "Notify" signal:

--- a/gconf/gconf-database-dbus.c
+++ b/gconf/gconf-database-dbus.c
@@ -929,7 +929,7 @@ gconf_database_dbus_notify_listeners (GConfDatabase    *db,
                                                      "Notify");
 
              dbus_message_append_args (message,
-                                       DBUS_TYPE_STRING, &db->object_path,
+                                       DBUS_TYPE_OBJECT_PATH, &db->object_path,
                                        DBUS_TYPE_STRING, &dir,

What about the other side:

I think it's handle_notify: "dbus_message_iter_get_basic (&iter, &db);" reading the source makes me think we don't need to change anything...(it treats an object path as a string in dbus_message_iter_get_basic)

Yes, only the emission needs to change to an object path, the listening side appears to be able to cope without code change.

commit d6f22138dc0362c706f7958bc3f42aef7af02d46
Author: Christian Persch <chpe@gnome.org>
Date:   Mon Aug 15 16:45:23 2011 +0100

    gconf-database-dbus: Fix notify listeners to also use an object path
    
    Fixes: https://bugzilla.gnome.org/show_bug.cgi?id=655656

Committd patches *might* have created some problems, *if* the reporter in bug 656867 is correct.