Lennart proposed a patch to fix the fact that missing objects, missing interfaces and missing methods all result in us raising UnknownMethod: http://lists.freedesktop.org/archives/dbus/2011-February/014107.html libdbus can only distinguish between UnknownObject and the others, so with Lennart's implementation it will never actually raise UnknownInterface itself, but the well-known string is made available for bindings and reimplementations. For dbus 1.4 I'd be happy to add the well-known strings to dbus-protocol.h if Lennart wants that, but I'd rather not take the rest of the patch. For dbus 1.5 I think the whole change looks good.
Created attachment 44295 [details] [review] [PATCH] connection: hook UnknownObject and UnknownInterface up where appropriate This makes use of UnknownInterface and UnknownObject where appropriate in the D-Bus core. --- bus/driver.c | 5 +- dbus/dbus-connection.c | 824 ++++++++++++++++++++++++----------------------- dbus/dbus-object-tree.c | 148 +++++---- dbus/dbus-object-tree.h | 7 +- 4 files changed, 497 insertions(+), 487 deletions(-)
Created attachment 44296 [details] [review] [PATCH] connection: hook UnknownObject and UnknownInterface up where appropriate This makes use of UnknownInterface and UnknownObject where appropriate in the D-Bus core. --- bus/driver.c | 5 +- dbus/dbus-connection.c | 824 ++++++++++++++++++++++++----------------------- dbus/dbus-object-tree.c | 148 +++++---- dbus/dbus-object-tree.h | 7 +- 4 files changed, 497 insertions(+), 487 deletions(-)
Created attachment 44297 [details] [review] [PATCH] protocol: introduce four new errors UnknownInterface, UnknownObject, UnknownProperty and PropertyReadOnly, as discussed on the ML. The first two are already used by various bindings, such as the Qt and Java binding, but have never been made official. --- dbus/dbus-protocol.h | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-)
The "four new errors" patch is suppsoed to be merged into dbus-1.4, the other into dbus-1.6.
(In reply to comment #3) > Created an attachment (id=44297) [details] > [PATCH] protocol: introduce four new errors > > > UnknownInterface, UnknownObject, UnknownProperty and PropertyReadOnly, > as discussed on the ML. > > The first two are already used by various bindings, such as the Qt and > Java binding, but have never been made official. > --- > dbus/dbus-protocol.h | 10 +++++++++- > 1 files changed, 9 insertions(+), 1 deletions(-) It would be helpful if there was a documented mentioning when to use each error but as discussed on the mailing list in another thread the D-Bus spec is probably not the place for that (a "best practices" addendum to the spec could be). So the patch looks good to me. (Btw, I've filed https://bugzilla.gnome.org/show_bug.cgi?id=644397 for GDBus to use these new errors.) Reviewed-by: David Zeuthen <davidz@redhat.com>
Review of attachment 44297 [details] [review]: Looks good for 1.4. Reviewed-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Review of attachment 44295 [details] [review]: Please don't change unrelated whitespace in a commit that makes functional changes. This is likely to make it harder to merge 1.4 into master. The rule of thumb I tend to use is that it's good to fix whitespace in lines you're changing anyway, or adjacent to those. I wouldn't object to separate whitespace-only patches (by which I mean: git diff --ignore-space-change outputs nothing) that could go into both 1.4 and master. I've marked the actual functional changes I found, which all look good. ::: dbus/dbus-connection.c @@ +4523,3 @@ dbus_int32_t reply_serial; DBusDispatchStatus status; + dbus_bool_t found_object; ++ @@ +4690,3 @@ + message, + &found_object); + ++ @@ +4729,2 @@ reply = dbus_message_new_error (message, + found_object ? DBUS_ERROR_UNKNOWN_METHOD : DBUS_ERROR_UNKNOWN_OBJECT, ++ ::: dbus/dbus-object-tree.c @@ +747,3 @@ _dbus_object_tree_dispatch_and_unlock (DBusObjectTree *tree, + DBusMessage *message, + dbus_bool_t *found_object) ++ @@ +795,3 @@ + if (found_object) + *found_object = !!subtree; + ++ @@ +1387,3 @@ } + result = _dbus_object_tree_dispatch_and_unlock (tree, message, NULL); ++ ::: dbus/dbus-object-tree.h @@ +44,3 @@ DBusHandlerResult _dbus_object_tree_dispatch_and_unlock (DBusObjectTree *tree, + DBusMessage *message, + dbus_bool_t *found_object); ++
The protocol.h part is now commited.
Created attachment 44332 [details] [review] new hookup patch Here's another try of the hookup patch. This time with --ignore-space-change and updated a little.
Review of attachment 44332 [details] [review]: Looks good to me, 1.5 only
Committed. Closing.
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.