From: Martin Xu <[email protected]>
---
obexd/client/session.c | 20 +++++++++++++++++---
1 file changed, 17 insertions(+), 3 deletions(-)
diff --git a/obexd/client/session.c b/obexd/client/session.c
index 361c921..f451093 100644
--- a/obexd/client/session.c
+++ b/obexd/client/session.c
@@ -675,13 +675,27 @@ static const GDBusMethodTable session_methods[] = {
{ }
};
+static char *target2str(const uint8_t *t)
+{
+ if (t == NULL)
+ return NULL;
+
+ return g_strdup_printf("%02X%02X%02X%02X-%02X%02X-%02X%02X-"
+ "%02X%02X-%02X%02X%02X%02X%02X%02X",
+ t[0], t[1], t[2], t[3], t[4], t[5], t[6], t[7],
+ t[8], t[9], t[10], t[11], t[12], t[13], t[14],
+ t[15]);
+}
+
static gboolean get_target(const GDBusPropertyTable *property,
DBusMessageIter *iter, void *data)
{
struct obc_session *session = data;
+ char *uuid;
- dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING,
- &session->driver->uuid);
+ uuid = target2str(session->driver->target);
+ dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
+ g_free(uuid);
return TRUE;
}
@@ -690,7 +704,7 @@ static gboolean target_exists(const GDBusPropertyTable *property, void *data)
{
struct obc_session *session = data;
- return session->driver->uuid != NULL;
+ return session->driver->target != NULL;
}
static const GDBusPropertyTable session_properties[] = {
--
1.7.10.4
Hi Luiz:
> -----Original Message-----
> From: [email protected] [mailto:linux-bluetooth-
> [email protected]] On Behalf Of Luiz Augusto von Dentz
> Sent: Friday, July 19, 2013 16:03
> To: [email protected]
> Cc: [email protected]
> Subject: Re: [PATCH] obex/session: Export the right target uuid
>
> Hi Martin,
>
> On Thu, Jul 18, 2013 at 5:38 AM, <[email protected]> wrote:
> > From: Martin Xu <[email protected]>
> >
> > ---
> > + dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
> > + g_free(uuid);
>
> Did you ever test this? I suspect this will crash with OPP because
I did. And session_target_exists() will check the target, if it is NULL above function will not be called.
Of course, we can do more check before call target2str().
>
> > return TRUE;
> > }
> > @@ -690,7 +704,7 @@ static gboolean target_exists(const
> > GDBusPropertyTable *property, void *data) {
> > struct obc_session *session = data;
> >
> > - return session->driver->uuid != NULL;
> > + return session->driver->target != NULL;
> > }
>
> I don't understand why you want to use the OBEX target here? There is
> no other reference to it instead we should be using what Device1.UUIDs
> provides that is Bluetooth 128 bits UUIDs in string format so the
> applications can use them directly. So I will repeat again, what needs
> to be done is reword the documentation to reflect what is implemented.
You know, the original patch send by me is just use Bluetooth 128 UUIDs.
But I found later that the client sessions is exporting the target UUIDs.
Please check code obexd/client/session.c line 690.
Target property of Client session does not need to align with Server session here?
>
> And let me repeat one last time, with ObjectManager you got signaled
> what interfaces are supported, that is much more important than the
It is not related with Objectmanager, I already use that.
But with the new added interface, how could user know what kind of obex session it is?
At many cases, UI just wants to display the transfer states of OPP.
In fact, I have sent patches to proposal several suggestions:
1. just use "OPP", "FTP" for the internal supported targetUUID for external profile to decide the profile
2. do not use target UUID just use Bluetooth 128 UUID
3. use target UUID
But to very solution, at least, we need to keep the definition consistent.
Hi Martin,
On Thu, Jul 18, 2013 at 5:38 AM, <[email protected]> wrote:
> From: Martin Xu <[email protected]>
>
> ---
> obexd/client/session.c | 20 +++++++++++++++++---
> 1 file changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/obexd/client/session.c b/obexd/client/session.c
> index 361c921..f451093 100644
> --- a/obexd/client/session.c
> +++ b/obexd/client/session.c
> @@ -675,13 +675,27 @@ static const GDBusMethodTable session_methods[] = {
> { }
> };
>
> +static char *target2str(const uint8_t *t)
> +{
> + if (t == NULL)
> + return NULL;
> +
> + return g_strdup_printf("%02X%02X%02X%02X-%02X%02X-%02X%02X-"
> + "%02X%02X-%02X%02X%02X%02X%02X%02X",
> + t[0], t[1], t[2], t[3], t[4], t[5], t[6], t[7],
> + t[8], t[9], t[10], t[11], t[12], t[13], t[14],
> + t[15]);
> +}
> +
> static gboolean get_target(const GDBusPropertyTable *property,
> DBusMessageIter *iter, void *data)
> {
> struct obc_session *session = data;
> + char *uuid;
>
> - dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING,
> - &session->driver->uuid);
> + uuid = target2str(session->driver->target);
> + dbus_message_iter_append_basic(iter, DBUS_TYPE_STRING, &uuid);
> + g_free(uuid);
Did you ever test this? I suspect this will crash with OPP because
there driver->target is NULL so target2str will return NULL as well so
you would be passing NULL to dbus_message_iter_append_basic which I
believe will cause a crash, so instead of fixing anything here this
would be introducing a bug.
> return TRUE;
> }
> @@ -690,7 +704,7 @@ static gboolean target_exists(const GDBusPropertyTable *property, void *data)
> {
> struct obc_session *session = data;
>
> - return session->driver->uuid != NULL;
> + return session->driver->target != NULL;
> }
I don't understand why you want to use the OBEX target here? There is
no other reference to it instead we should be using what Device1.UUIDs
provides that is Bluetooth 128 bits UUIDs in string format so the
applications can use them directly. So I will repeat again, what needs
to be done is reword the documentation to reflect what is implemented.
And let me repeat one last time, with ObjectManager you got signaled
what interfaces are supported, that is much more important than the
UUID, in fact perhaps it was a mistake to introduce such a thing since
the interfaces can be used directly but this can be handy for
non-standard profiles.
--
Luiz Augusto von Dentz