Re: [PATCH BlueZ 2/5] gdbus/client.c: don't assume the root object path is valid

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Steven,

On Wed, Dec 3, 2014 at 7:28 PM, Steven Walter <stevenrwalter@xxxxxxxxx> wrote:
> Hi Luiz
>
> On Wed, Dec 3, 2014 at 11:08 AM, Luiz Augusto von Dentz
> <luiz.dentz@xxxxxxxxx> wrote:
>> Hi Steven,
>>
>> On Wed, Dec 3, 2014 at 5:54 PM, Steven Walter <stevenrwalter@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Bluez Devel]     [Linux Wireless Networking]     [Linux Wireless Personal Area Networking]     [Linux ATH6KL]     [Linux USB Devel]     [Linux Media Drivers]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Big List of Linux Books]

  Powered by Linux