Return-Path: MIME-Version: 1.0 In-Reply-To: References: <1417622074-20023-1-git-send-email-stevenrwalter@gmail.com> <1417622074-20023-2-git-send-email-stevenrwalter@gmail.com> Date: Wed, 3 Dec 2014 20:32:26 +0200 Message-ID: Subject: Re: [PATCH BlueZ 2/5] gdbus/client.c: don't assume the root object path is valid From: Luiz Augusto von Dentz To: Steven Walter Cc: "linux-bluetooth@vger.kernel.org" , Arman Uguray Content-Type: text/plain; charset=UTF-8 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Steven, On Wed, Dec 3, 2014 at 7:28 PM, Steven Walter wrote: > Hi Luiz > > On Wed, Dec 3, 2014 at 11:08 AM, Luiz Augusto von Dentz > wrote: >> Hi Steven, >> >> On Wed, Dec 3, 2014 at 5:54 PM, Steven Walter wrote: >>> We should look for objects at the path the client has registered, not >>> hard-coded at root. In particular, python D-Bus objects will not >>> respond to requests directed at the root object path. >>> --- >>> gdbus/client.c | 2 +- >>> src/gatt-dbus.c | 2 +- >>> 2 files changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/gdbus/client.c b/gdbus/client.c >>> index eb68a0f..a270fc2 100644 >>> --- a/gdbus/client.c >>> +++ b/gdbus/client.c >>> @@ -1115,7 +1115,7 @@ static void get_managed_objects(GDBusClient *client) >>> if (client->get_objects_call != NULL) >>> return; >>> >>> - msg = dbus_message_new_method_call(client->service_name, "/", >>> + msg = dbus_message_new_method_call(client->service_name, client->base_path, >>> DBUS_INTERFACE_DBUS ".ObjectManager", >>> "GetManagedObjects"); >>> if (msg == NULL) >>> diff --git a/src/gatt-dbus.c b/src/gatt-dbus.c >>> index a04c38f..e61af15 100644 >>> --- a/src/gatt-dbus.c >>> +++ b/src/gatt-dbus.c >>> @@ -524,7 +524,7 @@ static struct external_service *external_service_new(DBusConnection *conn, >>> GDBusClient *client; >>> const char *sender = dbus_message_get_sender(msg); >>> >>> - client = g_dbus_client_new(conn, sender, "/"); >>> + client = g_dbus_client_new(conn, sender, path); >>> if (!client) >>> return NULL; >>> >>> -- >>> 1.9.1 >> >> I remember the spec had something regarding '/' must be present and >> the ObjectManager was only allowed on that because of that otherwise >> the clients have to know before hand what path are available which is >> the whole point of having ObjectManager to discover that. > > I don't read that in the spec. The specs says: > > "The root of each sub-tree implements this interface so other > applications can get all objects, interfaces and properties in a > single method call." > > Note "sub-tree", not "the root tree." The spec is also careful to > talk about the ObjectManager's object path, not assuming that the path > will be '/'. Well that means by having '/' you can have only one ObjectManager which simplify the implementation quite a bit, but I figure the real problem with not having '/' is that bindings that can register objects dynamically, such as in python I suppose, one could register a parent path to a sub-tree leaving a child path with ObjectManager that would duplicate signals related to ObjectManager which I believe is quite a major fault if the binding if it is allowing that to happen. Now that perhaps does not invalidate the patch if someone really know what they are doing, but I would first check if python is really doing the right thing here. Depending on the response we may even have to give some feedback to D-Bus folks regarding the use of sub-trees. > Regardless, we can always be less strict than the spec. Since we have > an object path, why wouldn't we use it? Sure but then we need to change quite a few other places that are still hardcoding '/' in gdbus code, also make sure gdbus changes are in a separate patch and does not break anything. -- Luiz Augusto von Dentz