* [PATCH 1/2] Fix memory leak if an error occurs when a data channel is reconnected.
From: Santiago Carot-Nemesio @ 2011-03-25 17:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
health/hdp.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 3c2dce1..7a4b219 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -514,9 +514,9 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
reply = g_dbus_create_error(dc_data->msg,
ERROR_INTERFACE ".HealthError",
"Cannot get file descriptor");
-
- reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD, &fd,
- DBUS_TYPE_INVALID);
+ else
+ reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD,
+ &fd, DBUS_TYPE_INVALID);
g_dbus_send_message(dc_data->conn, reply);
}
--
1.7.4.1
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
From: Johan Hedberg @ 2011-03-25 17:28 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=Yebkv+B=8WvaPkV1-Q+UifKX+UohN5p=977pq@mail.gmail.com>
Hi Lizardo,
On Fri, Mar 25, 2011, Anderson Lizardo wrote:
> > On Fri, Mar 25, 2011, Anderson Lizardo wrote:
> >> > +static u16 get_uuid16(u8 *uuid128)
> >> > +{
> >> > + u8 *b = bluetooth_base_uuid;
> >> > + u32 val;
> >> > + int i;
> >> > +
> >> > + for (i = 4; i < 16; i++) {
> >> > + if (b[i] != uuid128[i])
> >> > + return 0;
> >>
> >> Don't you need to check bytes 0 and 1 as well, i.e. only bytes 2 and 3
> >> are changed?
> >
> > The first four bytes can have any value for Bluetooth UUIDs. The purpose
> > of this part of the function is to determine whether it's a Bluetooth
> > UUID or not (i.e. derived from the Bluetooth base UUID), so the four
> > first bytes don't matter.
>
> At least for UUIDs used in attribute types I see this on the spec (page 1835):
>
> "Or, to put it more simply, the 16-bit Attribute UUID replaces the x’s
> in the follow-
> ing:
> 0000xxxx-0000-1000-8000-00805F9B34FB"
>
> I.e. , only third and fourth bytes being part of 16-bit UUID.
That's correct, and we're not contradicting each other here. The
function simply in the first stage doesn't care if it's a 32 or 16 bit
Bluetooth UUID and only after that it does the > 0xffff comparison.
Johan
^ permalink raw reply
* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
From: Anderson Lizardo @ 2011-03-25 17:26 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth; +Cc: Johan Hedberg
In-Reply-To: <AANLkTi=Yebkv+B=8WvaPkV1-Q+UifKX+UohN5p=977pq@mail.gmail.com>
Hi Johan,
On Fri, Mar 25, 2011 at 1:23 PM, Anderson Lizardo
<anderson.lizardo@openbossa.org> wrote:
> Hi Johan,
>
> On Fri, Mar 25, 2011 at 12:35 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
>> Hi Lizardo,
>>
>> On Fri, Mar 25, 2011, Anderson Lizardo wrote:
>>> > +static u16 get_uuid16(u8 *uuid128)
>>> > +{
>>> > + u8 *b = bluetooth_base_uuid;
>>> > + u32 val;
>>> > + int i;
>>> > +
>>> > + for (i = 4; i < 16; i++) {
>>> > + if (b[i] != uuid128[i])
>>> > + return 0;
>>>
>>> Don't you need to check bytes 0 and 1 as well, i.e. only bytes 2 and 3
>>> are changed?
>>
>> The first four bytes can have any value for Bluetooth UUIDs. The purpose
>> of this part of the function is to determine whether it's a Bluetooth
>> UUID or not (i.e. derived from the Bluetooth base UUID), so the four
>> first bytes don't matter.
>
> At least for UUIDs used in attribute types I see this on the spec (page 1835):
>
> "Or, to put it more simply, the 16-bit Attribute UUID replaces the x’s
> in the follow-
> ing:
> 0000xxxx-0000-1000-8000-00805F9B34FB"
>
> I.e. , only third and fourth bytes being part of 16-bit UUID.
Hmm, now I see you implicitly do what I was proposing by using:
+ if (val > 0xffff)
+ return 0;
So you can just ignore this comment :)
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
From: Anderson Lizardo @ 2011-03-25 17:23 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth; +Cc: Johan Hedberg
In-Reply-To: <20110325163549.GA9894@jh-x301>
Hi Johan,
On Fri, Mar 25, 2011 at 12:35 PM, Johan Hedberg <johan.hedberg@gmail.com> wrote:
> Hi Lizardo,
>
> On Fri, Mar 25, 2011, Anderson Lizardo wrote:
>> > +static u16 get_uuid16(u8 *uuid128)
>> > +{
>> > + u8 *b = bluetooth_base_uuid;
>> > + u32 val;
>> > + int i;
>> > +
>> > + for (i = 4; i < 16; i++) {
>> > + if (b[i] != uuid128[i])
>> > + return 0;
>>
>> Don't you need to check bytes 0 and 1 as well, i.e. only bytes 2 and 3
>> are changed?
>
> The first four bytes can have any value for Bluetooth UUIDs. The purpose
> of this part of the function is to determine whether it's a Bluetooth
> UUID or not (i.e. derived from the Bluetooth base UUID), so the four
> first bytes don't matter.
At least for UUIDs used in attribute types I see this on the spec (page 1835):
"Or, to put it more simply, the 16-bit Attribute UUID replaces the x’s
in the follow-
ing:
0000xxxx-0000-1000-8000-00805F9B34FB"
I.e. , only third and fourth bytes being part of 16-bit UUID.
>> > +static int update_eir(struct hci_dev *hdev)
>> > +{
>> > + struct hci_cp_write_eir cp;
>> > +
>> > + if (!(hdev->features[6] & LMP_EXT_INQ))
>> > + return 0;
>> > +
>> > + if (hdev->ssp_mode == 0)
>> > + return 0;
>> > +
>> > + if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
>> > + return 0;
>> > +
>> > + memset(&cp, 0, sizeof(cp));
>> > +
>> > + create_eir(hdev, cp.data);
>> > +
>> > + if (memcmp(cp.data, hdev->eir, sizeof(cp.data)) == 0)
>> > + return 0;
>>
>> What about making create_eir() return "int" and check for eir_len and
>> the end? That would avoid the memcmp() here.
>
> I'm not completely sure what you're proposing, but if it's to check for
> matching lengths of cp.data and hdev->eir before doing the memcmp then
> hdev would need a new eir_length member in addition to eir. Could be
> worth it, but it does slightly increase the complexity and since this is
> not a performance critical piece of code (won't be called too often) I'm
> not sure if it's worth it.
Actually I was proposing to get rid of the memcmp(), but now I see it
is necessary to not do HCI_OP_WRITE_EIR if EIR has not changed.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* [PATCH 3/3] Remove owner reference to request structure
From: Luiz Augusto von Dentz @ 2011-03-25 16:59 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1301072395-16787-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
This should avoid doing too much implicity and should improve the
readability of the code.
---
audio/transport.c | 100 ++++++++++++++++++++++++++++++++--------------------
1 files changed, 61 insertions(+), 39 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index e0390e5..3442e5f 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -52,7 +52,6 @@
struct media_request {
DBusMessage *msg;
guint id;
- struct media_owner *owner;
};
struct media_owner {
@@ -107,48 +106,36 @@ void media_transport_destroy(struct media_transport *transport)
g_free(path);
}
-static struct media_request *media_request_new(struct media_owner *owner,
- DBusMessage *msg)
+static struct media_request *media_request_create(DBusMessage *msg, guint id)
{
struct media_request *req;
req = g_new0(struct media_request, 1);
req->msg = dbus_message_ref(msg);
- req->owner = owner;
- owner->pending = req;
-
- return req;
-}
-
-static void media_request_free(struct media_request *req)
-{
- struct media_owner *owner = req->owner;
- struct media_transport *transport = owner->transport;
+ req->id = id;
- if (req->id)
- transport->cancel(owner->transport, req->id);
-
- if (req->msg)
- dbus_message_unref(req->msg);
+ DBG("Request created: method=%s id=%u", dbus_message_get_member(msg),
+ id);
- owner->pending = NULL;
- g_free(req);
+ return req;
}
-static void media_request_reply(struct media_request *req, int err)
+static void media_request_reply(struct media_request *req,
+ DBusConnection *conn, int err)
{
- struct media_owner *owner = req->owner;
- struct media_transport *transport = owner->transport;
DBusMessage *reply;
+ DBG("Request %s Reply %s", dbus_message_get_member(req->msg),
+ strerror(err));
+
if (!err)
reply = g_dbus_create_reply(req->msg, DBUS_TYPE_INVALID);
else
- reply = g_dbus_create_error(owner->pending->msg,
+ reply = g_dbus_create_error(req->msg,
ERROR_INTERFACE ".Failed",
"%s", strerror(err));
- g_dbus_send_message(transport->conn, reply);
+ g_dbus_send_message(conn, reply);
}
static gboolean media_transport_release(struct media_transport *transport,
@@ -167,12 +154,30 @@ static gboolean media_transport_release(struct media_transport *transport,
return TRUE;
}
+static void media_owner_remove(struct media_owner *owner,
+ struct media_request *req)
+{
+ struct media_transport *transport = owner->transport;
+
+ DBG("Owner %s Request %s", owner->name,
+ dbus_message_get_member(req->msg));
+
+ if (req->id)
+ transport->cancel(transport, req->id);
+
+ owner->pending = NULL;
+ if (req->msg)
+ dbus_message_unref(req->msg);
+
+ g_free(req);
+}
+
static void media_owner_free(struct media_owner *owner)
{
DBG("Owner %s", owner->name);
if (owner->pending)
- media_request_free(owner->pending);
+ media_owner_remove(owner, owner->pending);
g_free(owner->name);
g_free(owner->accesstype);
@@ -263,7 +268,7 @@ static void a2dp_resume_complete(struct avdtp *session,
if (ret == FALSE)
goto fail;
- media_request_free(req);
+ media_owner_remove(owner, req);
return;
@@ -307,7 +312,7 @@ static void a2dp_suspend_complete(struct avdtp *session,
/* Release always succeeds */
if (owner->pending) {
owner->pending->id = 0;
- media_request_reply(owner->pending, 0);
+ media_request_reply(owner->pending, transport->conn, 0);
}
a2dp_sep_unlock(sep, transport->session);
@@ -373,7 +378,7 @@ static void headset_resume_complete(struct audio_device *dev, void *user_data)
if (ret == FALSE)
goto fail;
- media_request_free(req);
+ media_owner_remove(owner, req);
return;
@@ -407,7 +412,7 @@ static void headset_suspend_complete(struct audio_device *dev, void *user_data)
/* Release always succeeds */
if (owner->pending) {
owner->pending->id = 0;
- media_request_reply(owner->pending, 0);
+ media_request_reply(owner->pending, transport->conn, 0);
}
headset_unlock(dev, HEADSET_LOCK_READ | HEADSET_LOCK_WRITE);
@@ -441,7 +446,7 @@ static void media_owner_exit(DBusConnection *connection, void *user_data)
owner->watch = 0;
if (owner->pending != NULL)
- media_request_free(owner->pending);
+ media_owner_remove(owner, owner->pending);
media_transport_remove(owner->transport, owner);
}
@@ -508,6 +513,15 @@ static struct media_owner *media_owner_create(DBusConnection *conn,
return owner;
}
+static void media_owner_add(struct media_owner *owner,
+ struct media_request *req)
+{
+ DBG("Owner %s Request %s", owner->name,
+ dbus_message_get_member(req->msg));
+
+ owner->pending = req;
+}
+
static struct media_owner *media_transport_find_owner(
struct media_transport *transport,
const char *name)
@@ -531,6 +545,7 @@ static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
struct media_owner *owner;
struct media_request *req;
const char *accesstype, *sender;
+ guint id;
if (!dbus_message_get_args(msg, NULL,
DBUS_TYPE_STRING, &accesstype,
@@ -547,13 +562,14 @@ static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
return btd_error_not_authorized(msg);
owner = media_owner_create(conn, msg, accesstype);
- req = media_request_new(owner, msg);
- req->id = transport->resume(transport, owner);
- if (req->id == 0) {
+ id = transport->resume(transport, owner);
+ if (id == 0) {
media_owner_free(owner);
- return NULL;
+ return btd_error_not_authorized(msg);
}
+ req = media_request_create(msg, id);
+ media_owner_add(owner, req);
media_transport_add(transport, owner);
return NULL;
@@ -579,6 +595,8 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
return btd_error_not_authorized(msg);
if (g_strcmp0(owner->accesstype, accesstype) == 0) {
+ guint id;
+
/* Not the last owner, no need to suspend */
if (g_slist_length(transport->owners) != 1) {
media_transport_remove(transport, owner);
@@ -591,15 +609,19 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
member = dbus_message_get_member(owner->pending->msg);
/* Cancel Acquire request if that exist */
if (g_str_equal(member, "Acquire"))
- media_request_free(owner->pending);
+ media_owner_remove(owner, owner->pending);
else
return btd_error_in_progress(msg);
}
- req = media_request_new(owner, msg);
- req->id = transport->suspend(transport, owner);
- if (req->id == 0)
+ id = transport->suspend(transport, owner);
+ if (id == 0) {
media_transport_remove(transport, owner);
+ return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+ }
+
+ req = media_request_create(msg, id);
+ media_owner_add(owner, req);
return NULL;
} else if (g_strstr_len(owner->accesstype, -1, accesstype) != NULL) {
--
1.7.1
^ permalink raw reply related
* [PATCH 2/3] Rework adding/removing owners to a transport
From: Luiz Augusto von Dentz @ 2011-03-25 16:59 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1301072395-16787-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Instead of directly add/remove and owner to transport list do it in a
separate function.
---
audio/media.c | 4 +-
audio/transport.c | 85 +++++++++++++++++++++++++++++++---------------------
audio/transport.h | 2 +-
3 files changed, 54 insertions(+), 37 deletions(-)
diff --git a/audio/media.c b/audio/media.c
index 4b389c6..0efb0a8 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -126,7 +126,7 @@ static void media_endpoint_remove(struct media_endpoint *endpoint)
media_endpoint_cancel(endpoint);
if (endpoint->transport)
- media_transport_remove(endpoint->transport);
+ media_transport_destroy(endpoint->transport);
g_dbus_remove_watch(adapter->conn, endpoint->watch);
g_free(endpoint->capabilities);
@@ -665,7 +665,7 @@ void media_endpoint_clear_configuration(struct media_endpoint *endpoint)
DBUS_TYPE_INVALID);
g_dbus_send_message(conn, msg);
done:
- media_transport_remove(endpoint->transport);
+ media_transport_destroy(endpoint->transport);
endpoint->transport = NULL;
}
diff --git a/audio/transport.c b/audio/transport.c
index 9a8fc22..e0390e5 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -95,7 +95,7 @@ struct media_transport {
DBusMessageIter *value);
};
-void media_transport_remove(struct media_transport *transport)
+void media_transport_destroy(struct media_transport *transport)
{
char *path;
@@ -167,30 +167,35 @@ static gboolean media_transport_release(struct media_transport *transport,
return TRUE;
}
-static void media_owner_remove(struct media_owner *owner)
+static void media_owner_free(struct media_owner *owner)
{
- struct media_transport *transport = owner->transport;
+ DBG("Owner %s", owner->name);
+
+ if (owner->pending)
+ media_request_free(owner->pending);
+
+ g_free(owner->name);
+ g_free(owner->accesstype);
+ g_free(owner);
+}
+
+static void media_transport_remove(struct media_transport *transport,
+ struct media_owner *owner)
+{
+ DBG("Transport %s Owner %s", transport->path, owner->name);
media_transport_release(transport, owner->accesstype);
+ transport->owners = g_slist_remove(transport->owners, owner);
+
if (owner->watch)
g_dbus_remove_watch(transport->conn, owner->watch);
- if (owner->pending)
- media_request_free(owner->pending);
-
- transport->owners = g_slist_remove(transport->owners, owner);
+ media_owner_free(owner);
/* Suspend if there is no longer any owner */
if (transport->owners == NULL && transport->in_use)
transport->suspend(transport, NULL);
-
- DBG("Owner removed: sender=%s accesstype=%s", owner->name,
- owner->accesstype);
-
- g_free(owner->name);
- g_free(owner->accesstype);
- g_free(owner);
}
static gboolean media_transport_set_fd(struct media_transport *transport,
@@ -210,7 +215,9 @@ static gboolean media_transport_set_fd(struct media_transport *transport,
static gboolean remove_owner(gpointer data)
{
- media_owner_remove(data);
+ struct media_owner *owner = data;
+
+ media_transport_remove(owner->transport, owner);
return FALSE;
}
@@ -305,7 +312,7 @@ static void a2dp_suspend_complete(struct avdtp *session,
a2dp_sep_unlock(sep, transport->session);
transport->in_use = FALSE;
- media_owner_remove(owner);
+ media_transport_remove(transport, owner);
}
static guint suspend_a2dp(struct media_transport *transport,
@@ -371,7 +378,7 @@ static void headset_resume_complete(struct audio_device *dev, void *user_data)
return;
fail:
- media_owner_remove(owner);
+ media_transport_remove(transport, owner);
}
static guint resume_headset(struct media_transport *transport,
@@ -405,7 +412,7 @@ static void headset_suspend_complete(struct audio_device *dev, void *user_data)
headset_unlock(dev, HEADSET_LOCK_READ | HEADSET_LOCK_WRITE);
transport->in_use = FALSE;
- media_owner_remove(owner);
+ media_transport_remove(transport, owner);
}
static guint suspend_headset(struct media_transport *transport,
@@ -436,7 +443,7 @@ static void media_owner_exit(DBusConnection *connection, void *user_data)
if (owner->pending != NULL)
media_request_free(owner->pending);
- media_owner_remove(owner);
+ media_transport_remove(owner->transport, owner);
}
static gboolean media_transport_acquire(struct media_transport *transport,
@@ -474,22 +481,26 @@ static gboolean media_transport_acquire(struct media_transport *transport,
return TRUE;
}
-static struct media_owner *media_owner_create(
- struct media_transport *transport,
- DBusMessage *msg,
- const char *accesstype)
+static void media_transport_add(struct media_transport *transport,
+ struct media_owner *owner)
+{
+ DBG("Transport %s Owner %s", transport->path, owner->name);
+ transport->owners = g_slist_append(transport->owners, owner);
+ owner->transport = transport;
+}
+
+static struct media_owner *media_owner_create(DBusConnection *conn,
+ DBusMessage *msg,
+ const char *accesstype)
{
struct media_owner *owner;
owner = g_new0(struct media_owner, 1);
- owner->transport = transport;
owner->name = g_strdup(dbus_message_get_sender(msg));
owner->accesstype = g_strdup(accesstype);
- owner->watch = g_dbus_add_disconnect_watch(transport->conn,
- owner->name,
+ owner->watch = g_dbus_add_disconnect_watch(conn, owner->name,
media_owner_exit,
owner, NULL);
- transport->owners = g_slist_append(transport->owners, owner);
DBG("Owner created: sender=%s accesstype=%s", owner->name,
accesstype);
@@ -535,11 +546,15 @@ static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
if (media_transport_acquire(transport, accesstype) == FALSE)
return btd_error_not_authorized(msg);
- owner = media_owner_create(transport, msg, accesstype);
+ owner = media_owner_create(conn, msg, accesstype);
req = media_request_new(owner, msg);
req->id = transport->resume(transport, owner);
- if (req->id == 0)
- media_owner_remove(owner);
+ if (req->id == 0) {
+ media_owner_free(owner);
+ return NULL;
+ }
+
+ media_transport_add(transport, owner);
return NULL;
}
@@ -566,7 +581,7 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
if (g_strcmp0(owner->accesstype, accesstype) == 0) {
/* Not the last owner, no need to suspend */
if (g_slist_length(transport->owners) != 1) {
- media_owner_remove(owner);
+ media_transport_remove(transport, owner);
return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
}
@@ -584,7 +599,7 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
req = media_request_new(owner, msg);
req->id = transport->suspend(transport, owner);
if (req->id == 0)
- media_owner_remove(owner);
+ media_transport_remove(transport, owner);
return NULL;
} else if (g_strstr_len(owner->accesstype, -1, accesstype) != NULL) {
@@ -774,9 +789,11 @@ static GDBusSignalTable transport_signals[] = {
static void media_transport_free(void *data)
{
struct media_transport *transport = data;
+ GSList *l;
+
+ for (l = transport->owners; l; l = l->next)
+ media_transport_remove(transport, l->data);
- g_slist_foreach(transport->owners, (GFunc) media_owner_remove,
- NULL);
g_slist_free(transport->owners);
if (transport->session)
diff --git a/audio/transport.h b/audio/transport.h
index b6c27b9..be4d666 100644
--- a/audio/transport.h
+++ b/audio/transport.h
@@ -30,7 +30,7 @@ struct media_transport *media_transport_create(DBusConnection *conn,
uint8_t *configuration,
size_t size);
-void media_transport_remove(struct media_transport *transport);
+void media_transport_destroy(struct media_transport *transport);
const char *media_transport_get_path(struct media_transport *transport);
void media_transport_update_delay(struct media_transport *transport,
uint16_t delay);
--
1.7.1
^ permalink raw reply related
* [PATCH 1/3] Make MediaTransport.Release asynchronous
From: Luiz Augusto von Dentz @ 2011-03-25 16:59 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
This make it possible for the owners to synchronize its state if the
transport is going to be suspended.
Note that client which don't want to wait for Release can just ignore/
not wait for its reply.
---
audio/transport.c | 168 +++++++++++++++++++++++++++++++++++++++++-----------
1 files changed, 132 insertions(+), 36 deletions(-)
diff --git a/audio/transport.c b/audio/transport.c
index 9961684..9a8fc22 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -49,7 +49,7 @@
#define MEDIA_TRANSPORT_INTERFACE "org.bluez.MediaTransport"
-struct acquire_request {
+struct media_request {
DBusMessage *msg;
guint id;
struct media_owner *owner;
@@ -57,7 +57,7 @@ struct acquire_request {
struct media_owner {
struct media_transport *transport;
- struct acquire_request *request;
+ struct media_request *pending;
char *name;
char *accesstype;
guint watch;
@@ -82,7 +82,8 @@ struct media_transport {
gboolean in_use;
guint (*resume) (struct media_transport *transport,
struct media_owner *owner);
- void (*suspend) (struct media_transport *transport);
+ guint (*suspend) (struct media_transport *transport,
+ struct media_owner *owner);
void (*cancel) (struct media_transport *transport,
guint id);
void (*get_properties) (
@@ -106,7 +107,20 @@ void media_transport_remove(struct media_transport *transport)
g_free(path);
}
-static void acquire_request_free(struct acquire_request *req)
+static struct media_request *media_request_new(struct media_owner *owner,
+ DBusMessage *msg)
+{
+ struct media_request *req;
+
+ req = g_new0(struct media_request, 1);
+ req->msg = dbus_message_ref(msg);
+ req->owner = owner;
+ owner->pending = req;
+
+ return req;
+}
+
+static void media_request_free(struct media_request *req)
{
struct media_owner *owner = req->owner;
struct media_transport *transport = owner->transport;
@@ -117,10 +131,26 @@ static void acquire_request_free(struct acquire_request *req)
if (req->msg)
dbus_message_unref(req->msg);
- owner->request = NULL;
+ owner->pending = NULL;
g_free(req);
}
+static void media_request_reply(struct media_request *req, int err)
+{
+ struct media_owner *owner = req->owner;
+ struct media_transport *transport = owner->transport;
+ DBusMessage *reply;
+
+ if (!err)
+ reply = g_dbus_create_reply(req->msg, DBUS_TYPE_INVALID);
+ else
+ reply = g_dbus_create_error(owner->pending->msg,
+ ERROR_INTERFACE ".Failed",
+ "%s", strerror(err));
+
+ g_dbus_send_message(transport->conn, reply);
+}
+
static gboolean media_transport_release(struct media_transport *transport,
const char *accesstype)
{
@@ -146,21 +176,14 @@ static void media_owner_remove(struct media_owner *owner)
if (owner->watch)
g_dbus_remove_watch(transport->conn, owner->watch);
- if (owner->request) {
- DBusMessage *reply = g_dbus_create_error(owner->request->msg,
- ERROR_INTERFACE ".Failed",
- "%s", strerror(EIO));
-
- g_dbus_send_message(transport->conn, reply);
-
- acquire_request_free(owner->request);
- }
+ if (owner->pending)
+ media_request_free(owner->pending);
transport->owners = g_slist_remove(transport->owners, owner);
- /* Suspend if the is no longer any owner */
- if (transport->owners == NULL)
- transport->suspend(transport);
+ /* Suspend if there is no longer any owner */
+ if (transport->owners == NULL && transport->in_use)
+ transport->suspend(transport, NULL);
DBG("Owner removed: sender=%s accesstype=%s", owner->name,
owner->accesstype);
@@ -196,7 +219,7 @@ static void a2dp_resume_complete(struct avdtp *session,
struct avdtp_error *err, void *user_data)
{
struct media_owner *owner = user_data;
- struct acquire_request *req = owner->request;
+ struct media_request *req = owner->pending;
struct media_transport *transport = owner->transport;
struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
struct avdtp_stream *stream;
@@ -233,6 +256,8 @@ static void a2dp_resume_complete(struct avdtp *session,
if (ret == FALSE)
goto fail;
+ media_request_free(req);
+
return;
fail:
@@ -265,13 +290,38 @@ done:
owner);
}
-static void suspend_a2dp(struct media_transport *transport)
+static void a2dp_suspend_complete(struct avdtp *session,
+ struct avdtp_error *err, void *user_data)
{
- struct media_endpoint *endpoint = transport->endpoint;
- struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
+ struct media_owner *owner = user_data;
+ struct media_transport *transport = owner->transport;
+ struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
+
+ /* Release always succeeds */
+ if (owner->pending) {
+ owner->pending->id = 0;
+ media_request_reply(owner->pending, 0);
+ }
a2dp_sep_unlock(sep, transport->session);
transport->in_use = FALSE;
+ media_owner_remove(owner);
+}
+
+static guint suspend_a2dp(struct media_transport *transport,
+ struct media_owner *owner)
+{
+ struct media_endpoint *endpoint = transport->endpoint;
+ struct a2dp_sep *sep = media_endpoint_get_sep(endpoint);
+
+ if (!owner) {
+ a2dp_sep_unlock(sep, transport->session);
+ transport->in_use = FALSE;
+ return 0;
+ }
+
+ return a2dp_suspend(transport->session, sep, a2dp_suspend_complete,
+ owner);
}
static void cancel_a2dp(struct media_transport *transport, guint id)
@@ -282,7 +332,7 @@ static void cancel_a2dp(struct media_transport *transport, guint id)
static void headset_resume_complete(struct audio_device *dev, void *user_data)
{
struct media_owner *owner = user_data;
- struct acquire_request *req = owner->request;
+ struct media_request *req = owner->pending;
struct media_transport *transport = owner->transport;
int fd;
uint16_t imtu, omtu;
@@ -316,6 +366,8 @@ static void headset_resume_complete(struct audio_device *dev, void *user_data)
if (ret == FALSE)
goto fail;
+ media_request_free(req);
+
return;
fail:
@@ -340,12 +392,34 @@ done:
owner);
}
-static void suspend_headset(struct media_transport *transport)
+static void headset_suspend_complete(struct audio_device *dev, void *user_data)
{
- struct audio_device *device = transport->device;
+ struct media_owner *owner = user_data;
+ struct media_transport *transport = owner->transport;
+
+ /* Release always succeeds */
+ if (owner->pending) {
+ owner->pending->id = 0;
+ media_request_reply(owner->pending, 0);
+ }
- headset_unlock(device, HEADSET_LOCK_READ | HEADSET_LOCK_WRITE);
+ headset_unlock(dev, HEADSET_LOCK_READ | HEADSET_LOCK_WRITE);
transport->in_use = FALSE;
+ media_owner_remove(owner);
+}
+
+static guint suspend_headset(struct media_transport *transport,
+ struct media_owner *owner)
+{
+ struct audio_device *device = transport->device;
+
+ if (!owner) {
+ headset_unlock(device, HEADSET_LOCK_READ | HEADSET_LOCK_WRITE);
+ transport->in_use = FALSE;
+ return 0;
+ }
+
+ return headset_suspend_stream(device, headset_suspend_complete, owner);
}
static void cancel_headset(struct media_transport *transport, guint id)
@@ -358,8 +432,9 @@ static void media_owner_exit(DBusConnection *connection, void *user_data)
struct media_owner *owner = user_data;
owner->watch = 0;
- if (owner->request != NULL)
- acquire_request_free(owner->request);
+
+ if (owner->pending != NULL)
+ media_request_free(owner->pending);
media_owner_remove(owner);
}
@@ -443,7 +518,7 @@ static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
{
struct media_transport *transport = data;
struct media_owner *owner;
- struct acquire_request *req;
+ struct media_request *req;
const char *accesstype, *sender;
if (!dbus_message_get_args(msg, NULL,
@@ -461,11 +536,8 @@ static DBusMessage *acquire(DBusConnection *conn, DBusMessage *msg,
return btd_error_not_authorized(msg);
owner = media_owner_create(transport, msg, accesstype);
- req = g_new0(struct acquire_request, 1);
- req->msg = dbus_message_ref(msg);
- req->owner = owner;
+ req = media_request_new(owner, msg);
req->id = transport->resume(transport, owner);
- owner->request = req;
if (req->id == 0)
media_owner_remove(owner);
@@ -478,6 +550,7 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
struct media_transport *transport = data;
struct media_owner *owner;
const char *accesstype, *sender;
+ struct media_request *req;
if (!dbus_message_get_args(msg, NULL,
DBUS_TYPE_STRING, &accesstype,
@@ -490,9 +563,31 @@ static DBusMessage *release(DBusConnection *conn, DBusMessage *msg,
if (owner == NULL)
return btd_error_not_authorized(msg);
- if (g_strcmp0(owner->accesstype, accesstype) == 0)
- media_owner_remove(owner);
- else if (g_strstr_len(owner->accesstype, -1, accesstype) != NULL) {
+ if (g_strcmp0(owner->accesstype, accesstype) == 0) {
+ /* Not the last owner, no need to suspend */
+ if (g_slist_length(transport->owners) != 1) {
+ media_owner_remove(owner);
+ return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
+ }
+
+ if (owner->pending) {
+ const char *member;
+
+ member = dbus_message_get_member(owner->pending->msg);
+ /* Cancel Acquire request if that exist */
+ if (g_str_equal(member, "Acquire"))
+ media_request_free(owner->pending);
+ else
+ return btd_error_in_progress(msg);
+ }
+
+ req = media_request_new(owner, msg);
+ req->id = transport->suspend(transport, owner);
+ if (req->id == 0)
+ media_owner_remove(owner);
+
+ return NULL;
+ } else if (g_strstr_len(owner->accesstype, -1, accesstype) != NULL) {
media_transport_release(transport, accesstype);
g_strdelimit(owner->accesstype, accesstype, ' ');
} else
@@ -665,7 +760,8 @@ static GDBusMethodTable transport_methods[] = {
{ "GetProperties", "", "a{sv}", get_properties },
{ "Acquire", "s", "h", acquire,
G_DBUS_METHOD_FLAG_ASYNC},
- { "Release", "s", "", release },
+ { "Release", "s", "", release,
+ G_DBUS_METHOD_FLAG_ASYNC},
{ "SetProperty", "sv", "", set_property },
{ },
};
--
1.7.1
^ permalink raw reply related
* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
From: Johan Hedberg @ 2011-03-25 16:49 UTC (permalink / raw)
To: Anderson Lizardo, linux-bluetooth
In-Reply-To: <20110325163549.GA9894@jh-x301>
Hi,
On Fri, Mar 25, 2011, Johan Hedberg wrote:
> > > + }
> > > +
> > > + memcpy(&val, uuid128, 4);
> > > +
> > > + val = be32_to_cpu(val);
> >
> > I noticed just know the those UUID handling functions from management
> > API are assuming UUIDs in network order. We will need to change this,
> > or take extra care when reusing them for Advertising Data and GATT
> > uuids.
>
> Right. This code is pretty much a copy of what user space already has
> (hciops.c), and that code is relying on SDP (i.e. network) byte order
> UUID-128s.
Actually now that I think about it, for consistency with everything else
in the management interface the byte order, at least in the management
protocol, should probably stay little endian (mimicing HCI). We *could*
store the UUIDs in hdev->uuids in host byte order, but then we'll need
to add all the byte-order dependent code that we've recently created for
user space with bt_uuid.
The really important part is the management protocol since that's what's
visible to user space. The internal storage of these UUIDs in the kernel
is secondary. Personally, I'd like to avoid any 128-bit byte order
conversions if possible but I can see how this can be argued in both
ways.
Johan
^ permalink raw reply
* Re: [PATCH] Emit missing signal when data channel is reconnected.
From: Santiago Carot @ 2011-03-25 16:46 UTC (permalink / raw)
To: Santiago Carot, linux-bluetooth; +Cc: Johan Hedberg
In-Reply-To: <20110325161532.GA9649@jh-x301>
Hi,
2011/3/25 Johan Hedberg <johan.hedberg@gmail.com>:
> Hi,
>
> On Fri, Mar 25, 2011, Santiago Carot wrote:
>> >> diff --git a/health/hdp.c b/health/hdp.c
>> >> index 3c2dce1..b06fe17 100644
>> >> --- a/health/hdp.c
>> >> +++ b/health/hdp.c
>> >> @@ -510,14 +510,23 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
>> >> }
>> >>
>> >> fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
>> >> - if (fd < 0)
>> >> + if (fd < 0) {
>> >> reply = g_dbus_create_error(dc_data->msg,
>> >> ERROR_INTERFACE ".HealthError",
>> >> "Cannot get file descriptor");
>> >> + g_dbus_send_message(dc_data->conn, reply);
>> >> + return;
>> >> + }
>> >>
>>
>> This is not a memory leak fix, if fd is less than 0, then reconnection
>> was not succesfully and then we reply with an error response to the
>> application but we dont not emit the CahnnelConnected signal. (return
>> statement).
>
> The code (before your patch) was assigning a value to reply and then
> right afterwards assigning another value to it without freeing it in
> between:
>
>> >> reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD, &fd,
>> >> DBUS_TYPE_INVALID);
>
> How is that not a memory leak? (not to mention that g_dbus_send_message
> isn't called to the error message that you create). Looks to me like
> there was an "else" statement missing or something similar.
>
Sorry,
I didn't understood what you meant. I though that you was referring to
the pach but you was referring to the old code. Now I see.
You are right, I'm going to split it in two patches right away.
^ permalink raw reply
* [PATCH 2/2] Add ERROR response on AT+BLDN command for maemo6
From: Dmitriy Paliy @ 2011-03-25 16:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1301071449-22230-1-git-send-email-dmitriy.paliy@nokia.com>
Response on AT+BLDN command in maemo6 telephony driver is added to be
sent after confirmation from csd back-end. Both ERROR or OK codes are
sent only after asynchronous response on D-Bus call is available.
---
audio/telephony-maemo6.c | 26 +++++++++++++++++++++++---
1 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
index 9650cba..0cef7dd 100644
--- a/audio/telephony-maemo6.c
+++ b/audio/telephony-maemo6.c
@@ -618,6 +618,28 @@ static void remove_pending(DBusPendingCall *call)
pending_req_finalize(req);
}
+static void last_number_call_reply(DBusPendingCall *call, void *user_data)
+{
+ DBusError err;
+ DBusMessage *reply;
+ void *telephony_device = user_data;
+
+ reply = dbus_pending_call_steal_reply(call);
+
+ dbus_error_init(&err);
+ if (dbus_set_error_from_message(&err, reply)) {
+ error("csd replied with an error: %s, %s",
+ err.name, err.message);
+ dbus_error_free(&err);
+ telephony_dial_number_rsp(telephony_device,
+ CME_ERROR_AG_FAILURE);
+ } else
+ telephony_dial_number_rsp(telephony_device, CME_ERROR_NONE);
+
+ dbus_message_unref(reply);
+ remove_pending(call);
+}
+
void telephony_last_dialed_number_req(void *telephony_device)
{
int ret;
@@ -626,13 +648,11 @@ void telephony_last_dialed_number_req(void *telephony_device)
ret = send_method_call(CSD_CALL_BUS_NAME, CSD_CALL_PATH,
CSD_CALL_INTERFACE, "CreateFromLast",
- NULL, NULL,
+ last_number_call_reply, telephony_device,
DBUS_TYPE_INVALID);
if (ret < 0)
telephony_dial_number_rsp(telephony_device,
CME_ERROR_AG_FAILURE);
- else
- telephony_dial_number_rsp(telephony_device, CME_ERROR_NONE);
}
static const char *memory_dial_lookup(int location)
--
1.7.1
^ permalink raw reply related
* [PATCH 1/2] Add handling of pending D-Bus calls in maemo6
From: Dmitriy Paliy @ 2011-03-25 16:44 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Dmitriy Paliy
In-Reply-To: <1301071449-22230-1-git-send-email-dmitriy.paliy@nokia.com>
Handling of pending D-Bus calls to csd back-end is added maemo6
telephony driver.
---
audio/telephony-maemo6.c | 65 +++++++++++++++++++++++++++++++++++++++------
1 files changed, 56 insertions(+), 9 deletions(-)
diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
index 55f6a30..9650cba 100644
--- a/audio/telephony-maemo6.c
+++ b/audio/telephony-maemo6.c
@@ -134,6 +134,11 @@ static struct {
.signal_bars = 0,
};
+struct pending_req {
+ DBusPendingCall *call;
+ void *user_data;
+};
+
static int get_property(const char *iface, const char *prop);
static DBusConnection *connection = NULL;
@@ -443,10 +448,31 @@ void telephony_device_connected(void *telephony_device)
}
}
+static void pending_req_finalize(struct pending_req *req)
+{
+ if (!dbus_pending_call_get_completed(req->call))
+ dbus_pending_call_cancel(req->call);
+
+ dbus_pending_call_unref(req->call);
+ g_free(req);
+}
+
+static void remove_pending_by_data(gpointer data, gpointer user_data)
+{
+ struct pending_req *req = data;
+
+ if (req->user_data == user_data) {
+ pending = g_slist_remove(pending, req);
+ pending_req_finalize(req);
+ }
+}
+
void telephony_device_disconnected(void *telephony_device)
{
DBG("telephony-maemo6: device %p disconnected", telephony_device);
events_enabled = FALSE;
+
+ g_slist_foreach(pending, remove_pending_by_data, telephony_device);
}
void telephony_event_reporting_req(void *telephony_device, int ind)
@@ -529,6 +555,7 @@ static int send_method_call(const char *dest, const char *path,
DBusMessage *msg;
DBusPendingCall *call;
va_list args;
+ struct pending_req *req;
msg = dbus_message_new_method_call(dest, path, interface, method);
if (!msg) {
@@ -558,12 +585,39 @@ static int send_method_call(const char *dest, const char *path,
}
dbus_pending_call_set_notify(call, cb, user_data, NULL);
- pending = g_slist_prepend(pending, call);
+
+ req = g_new0(struct pending_req, 1);
+ req->call = call;
+ req->user_data = user_data;
+
+ pending = g_slist_prepend(pending, req);
dbus_message_unref(msg);
return 0;
}
+static struct pending_req *find_request(const DBusPendingCall *call)
+{
+ GSList *l;
+
+ for (l = pending; l; l = l->next) {
+ struct pending_req *req = l->data;
+
+ if (req->call == call)
+ return req;
+ }
+
+ return NULL;
+}
+
+static void remove_pending(DBusPendingCall *call)
+{
+ struct pending_req *req = find_request(call);
+
+ pending = g_slist_remove(pending, req);
+ pending_req_finalize(req);
+}
+
void telephony_last_dialed_number_req(void *telephony_device)
{
int ret;
@@ -1295,12 +1349,6 @@ static gboolean iter_get_basic_args(DBusMessageIter *iter,
return type == DBUS_TYPE_INVALID ? TRUE : FALSE;
}
-static void remove_pending(DBusPendingCall *call)
-{
- pending = g_slist_remove(pending, call);
- dbus_pending_call_unref(call);
-}
-
static void hal_battery_level_reply(DBusPendingCall *call, void *user_data)
{
DBusError err;
@@ -1910,8 +1958,7 @@ void telephony_exit(void)
g_slist_free(calls);
calls = NULL;
- g_slist_foreach(pending, (GFunc) dbus_pending_call_cancel, NULL);
- g_slist_foreach(pending, (GFunc) dbus_pending_call_unref, NULL);
+ g_slist_foreach(pending, (GFunc) pending_req_finalize, NULL);
g_slist_free(pending);
pending = NULL;
--
1.7.1
^ permalink raw reply related
* [PATCH 0/2 v2] Add handling of pending D-Bus calls in maemo6
From: Dmitriy Paliy @ 2011-03-25 16:44 UTC (permalink / raw)
To: linux-bluetooth
Hi,
Canceling of requests to csd is done in more correct way in this proposal.
BR,
Dmitriy
^ permalink raw reply
* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
From: Johan Hedberg @ 2011-03-25 16:35 UTC (permalink / raw)
To: Anderson Lizardo; +Cc: linux-bluetooth
In-Reply-To: <AANLkTinhC8-1fe0KYBb+Tz-x7RZSC3FtometdNw05xgX@mail.gmail.com>
Hi Lizardo,
On Fri, Mar 25, 2011, Anderson Lizardo wrote:
> > +static u16 get_uuid16(u8 *uuid128)
> > +{
> > + u8 *b = bluetooth_base_uuid;
> > + u32 val;
> > + int i;
> > +
> > + for (i = 4; i < 16; i++) {
> > + if (b[i] != uuid128[i])
> > + return 0;
>
> Don't you need to check bytes 0 and 1 as well, i.e. only bytes 2 and 3
> are changed?
The first four bytes can have any value for Bluetooth UUIDs. The purpose
of this part of the function is to determine whether it's a Bluetooth
UUID or not (i.e. derived from the Bluetooth base UUID), so the four
first bytes don't matter.
> > + }
> > +
> > + memcpy(&val, uuid128, 4);
> > +
> > + val = be32_to_cpu(val);
>
> I noticed just know the those UUID handling functions from management
> API are assuming UUIDs in network order. We will need to change this,
> or take extra care when reusing them for Advertising Data and GATT
> uuids.
Right. This code is pretty much a copy of what user space already has
(hciops.c), and that code is relying on SDP (i.e. network) byte order
UUID-128s.
> > + /* Group all UUID16 types */
> > + list_for_each(p, &hdev->uuids) {
> > + struct bt_uuid *uuid = list_entry(p, struct bt_uuid, list);
> > + u16 uuid16;
> > +
> > + uuid16 = get_uuid16(uuid->uuid);
> > + if (uuid16 == 0)
> > + return;
> > +
> > + if (uuid16 < 0x1100)
> > + continue;
> > +
> > + if (uuid16 == PNP_INFO_SVCLASS_ID)
> > + continue;
> > +
> > + /* Stop if not enough space to put next UUID */
> > + if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
> > + truncated = 1;
> > + break;
> > + }
> > +
> > + /* Check for duplicates */
> > + for (i = 0; uuid16_list[i] != 0; i++)
> > + if (uuid16_list[i] == uuid16)
> > + break;
> > +
> > + if (uuid16_list[i] == 0) {
> > + uuid16_list[i] = uuid16;
> > + eir_len += sizeof(u16);
> > + }
> > + }
> > +
> > + if (uuid16_list[0] != 0) {
> > + u8 *length = ptr;
> > +
> > + /* EIR Data type */
> > + ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
> > +
> > + ptr += 2;
> > + eir_len += 2;
>
> Here you increment eir_len but does not use it anymore. Maybe it would
> be nice to add some assert (BUG_ON ?) on it just to check for
> programming errors if someone tries to change this code later.
This is a straight copy from user space with UUID-128, TX power and
Device ID info support stripped away. Once those get added the eir_len
variable will be important later in the function.
> > +static int update_eir(struct hci_dev *hdev)
> > +{
> > + struct hci_cp_write_eir cp;
> > +
> > + if (!(hdev->features[6] & LMP_EXT_INQ))
> > + return 0;
> > +
> > + if (hdev->ssp_mode == 0)
> > + return 0;
> > +
> > + if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
> > + return 0;
> > +
> > + memset(&cp, 0, sizeof(cp));
> > +
> > + create_eir(hdev, cp.data);
> > +
> > + if (memcmp(cp.data, hdev->eir, sizeof(cp.data)) == 0)
> > + return 0;
>
> What about making create_eir() return "int" and check for eir_len and
> the end? That would avoid the memcmp() here.
I'm not completely sure what you're proposing, but if it's to check for
matching lengths of cp.data and hdev->eir before doing the memcmp then
hdev would need a new eir_length member in addition to eir. Could be
worth it, but it does slightly increase the complexity and since this is
not a performance critical piece of code (won't be called too often) I'm
not sure if it's worth it.
Johan
^ permalink raw reply
* Re: [PATCH v2 0/3] Another try for the Sixaxis plugin
From: Antonio Ospite @ 2011-03-25 16:27 UTC (permalink / raw)
To: Jim Paris
Cc: Bastien Nocera, linux-bluetooth, linux-input, Ranulf Doswell,
Pascal A . Brisset, Marcin Tolysz, Christian Birchinger,
Filipe Lopes, Alan Ott, Mikko Virkkila
In-Reply-To: <20110325153105.GA26039@psychosis.jim.sh>
[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]
On Fri, 25 Mar 2011 11:31:05 -0400
Jim Paris <jim@jtan.com> wrote:
> Antonio Ospite wrote:
> > With regard to that, I'd like to see some USB dumps of a Sixaxis
> > talking with GameOS to check if the PS3 can turn off BT on the
> > controller explicitly, can anyone help here? (Pascal? Do you have
> > access to a USB analyzer?):
> >
> > The scenarios I am interested in are:
> > 1. Connect a non-paired Sixaxis to the PS3 via USB
> > 2. Connect an already paired Sixaxis to the PS3 via USB
> > 3. Connect a Sixaxis already associated via BT to a PS3 via USB
>
> Hi Antonio,
>
> You asked me for this a while ago, sorry it took so long. I ran some
> tests and put the log files here:
> http://ps3.jim.sh/sixaxis/dumps/
>
Thanks a lot Jim, now I know what to do if I got bored this weekend.
[...]
> I also did some quick captures of pairing the keypad (CECHZK1UC) and
> original headset (CECHYA-0075). They are being paired with 00:13:a9:74:fe:57.
> http://ps3.jim.sh/sixaxis/dumps/
>
> -jim
>
If I get to taking a look at those I could ask Bastien to test some
stuff too :)
Thanks,
Antonio
--
Antonio Ospite
http://ao2.it
PGP public key ID: 0x4553B001
A: Because it messes up the order in which people normally read text.
See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?
[-- Attachment #2: Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] Emit missing signal when data channel is reconnected.
From: Johan Hedberg @ 2011-03-25 16:15 UTC (permalink / raw)
To: Santiago Carot; +Cc: linux-bluetooth
In-Reply-To: <AANLkTimEenNDyt3Q6wSoeqiA83hQ4Xth=2dUTf1J9knH@mail.gmail.com>
Hi,
On Fri, Mar 25, 2011, Santiago Carot wrote:
> >> diff --git a/health/hdp.c b/health/hdp.c
> >> index 3c2dce1..b06fe17 100644
> >> --- a/health/hdp.c
> >> +++ b/health/hdp.c
> >> @@ -510,14 +510,23 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
> >> }
> >>
> >> fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
> >> - if (fd < 0)
> >> + if (fd < 0) {
> >> reply = g_dbus_create_error(dc_data->msg,
> >> ERROR_INTERFACE ".HealthError",
> >> "Cannot get file descriptor");
> >> + g_dbus_send_message(dc_data->conn, reply);
> >> + return;
> >> + }
> >>
>
> This is not a memory leak fix, if fd is less than 0, then reconnection
> was not succesfully and then we reply with an error response to the
> application but we dont not emit the CahnnelConnected signal. (return
> statement).
The code (before your patch) was assigning a value to reply and then
right afterwards assigning another value to it without freeing it in
between:
> >> reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD, &fd,
> >> DBUS_TYPE_INVALID);
How is that not a memory leak? (not to mention that g_dbus_send_message
isn't called to the error message that you create). Looks to me like
there was an "else" statement missing or something similar.
Johan
^ permalink raw reply
* Re: [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
From: Anderson Lizardo @ 2011-03-25 15:36 UTC (permalink / raw)
To: johan.hedberg; +Cc: linux-bluetooth
In-Reply-To: <1301059544-6200-1-git-send-email-johan.hedberg@gmail.com>
Hi Johan,
On Fri, Mar 25, 2011 at 9:25 AM, <johan.hedberg@gmail.com> wrote:
> +static u8 bluetooth_base_uuid[] = {
> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
> + 0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB,
> +};
> +
> +static u16 get_uuid16(u8 *uuid128)
> +{
> + u8 *b = bluetooth_base_uuid;
> + u32 val;
> + int i;
> +
> + for (i = 4; i < 16; i++) {
> + if (b[i] != uuid128[i])
> + return 0;
Don't you need to check bytes 0 and 1 as well, i.e. only bytes 2 and 3
are changed?
> + }
> +
> + memcpy(&val, uuid128, 4);
> +
> + val = be32_to_cpu(val);
I noticed just know the those UUID handling functions from management
API are assuming UUIDs in network order. We will need to change this,
or take extra care when reusing them for Advertising Data and GATT
uuids.
> + if (val > 0xffff)
> + return 0;
> +
> + return (u16) val;
> +}
> +
> +static void create_eir(struct hci_dev *hdev, u8 *data)
> +{
> + u8 *ptr = data;
> + u16 eir_len = 0;
> + u16 uuid16_list[HCI_MAX_EIR_LENGTH / sizeof(u16)];
> + int i, truncated = 0;
> + struct list_head *p;
> + size_t name_len;
> +
> + name_len = strlen(hdev->dev_name);
> +
> + if (name_len > 0) {
> + /* EIR Data type */
> + if (name_len > 48) {
> + name_len = 48;
> + ptr[1] = EIR_NAME_SHORT;
> + } else
> + ptr[1] = EIR_NAME_COMPLETE;
> +
> + /* EIR Data length */
> + ptr[0] = name_len + 1;
> +
> + memcpy(ptr + 2, hdev->dev_name, name_len);
> +
> + eir_len += (name_len + 2);
> + ptr += (name_len + 2);
> + }
> +
> + memset(uuid16_list, 0, sizeof(uuid16_list));
> +
> + /* Group all UUID16 types */
> + list_for_each(p, &hdev->uuids) {
> + struct bt_uuid *uuid = list_entry(p, struct bt_uuid, list);
> + u16 uuid16;
> +
> + uuid16 = get_uuid16(uuid->uuid);
> + if (uuid16 == 0)
> + return;
> +
> + if (uuid16 < 0x1100)
> + continue;
> +
> + if (uuid16 == PNP_INFO_SVCLASS_ID)
> + continue;
> +
> + /* Stop if not enough space to put next UUID */
> + if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
> + truncated = 1;
> + break;
> + }
> +
> + /* Check for duplicates */
> + for (i = 0; uuid16_list[i] != 0; i++)
> + if (uuid16_list[i] == uuid16)
> + break;
> +
> + if (uuid16_list[i] == 0) {
> + uuid16_list[i] = uuid16;
> + eir_len += sizeof(u16);
> + }
> + }
> +
> + if (uuid16_list[0] != 0) {
> + u8 *length = ptr;
> +
> + /* EIR Data type */
> + ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
> +
> + ptr += 2;
> + eir_len += 2;
Here you increment eir_len but does not use it anymore. Maybe it would
be nice to add some assert (BUG_ON ?) on it just to check for
programming errors if someone tries to change this code later.
> +
> + for (i = 0; uuid16_list[i] != 0; i++) {
> + *ptr++ = (uuid16_list[i] & 0x00ff);
> + *ptr++ = (uuid16_list[i] & 0xff00) >> 8;
> + }
> +
> + /* EIR Data length */
> + *length = (i * sizeof(u16)) + 1;
> + }
> +}
> +
> +static int update_eir(struct hci_dev *hdev)
> +{
> + struct hci_cp_write_eir cp;
> +
> + if (!(hdev->features[6] & LMP_EXT_INQ))
> + return 0;
> +
> + if (hdev->ssp_mode == 0)
> + return 0;
> +
> + if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
> + return 0;
> +
> + memset(&cp, 0, sizeof(cp));
> +
> + create_eir(hdev, cp.data);
> +
> + if (memcmp(cp.data, hdev->eir, sizeof(cp.data)) == 0)
> + return 0;
What about making create_eir() return "int" and check for eir_len and
the end? That would avoid the memcmp() here.
Regards,
--
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil
^ permalink raw reply
* Re: [PATCH v2 0/3] Another try for the Sixaxis plugin
From: Jim Paris @ 2011-03-25 15:31 UTC (permalink / raw)
To: Antonio Ospite
Cc: Bastien Nocera, linux-bluetooth, linux-input, Ranulf Doswell,
Pascal A . Brisset, Marcin Tolysz, Christian Birchinger,
Filipe Lopes, Alan Ott, Mikko Virkkila
In-Reply-To: <20110324183247.3291e59e.ospite@studenti.unina.it>
Antonio Ospite wrote:
> With regard to that, I'd like to see some USB dumps of a Sixaxis
> talking with GameOS to check if the PS3 can turn off BT on the
> controller explicitly, can anyone help here? (Pascal? Do you have
> access to a USB analyzer?):
>
> The scenarios I am interested in are:
> 1. Connect a non-paired Sixaxis to the PS3 via USB
> 2. Connect an already paired Sixaxis to the PS3 via USB
> 3. Connect a Sixaxis already associated via BT to a PS3 via USB
Hi Antonio,
You asked me for this a while ago, sorry it took so long. I ran some
tests and put the log files here:
http://ps3.jim.sh/sixaxis/dumps/
There are comments in the file:
$ grep Comment sixaxis-pairing-tests-abbreviated.csv
0,,2,0:03.145.728,,,,,,Comment,,"1. Controller is a DS3 model CECHZC2U, bdaddr 00:24:33:E6:2C:3C"
0,,3,0:04.893.354,,,,,,Comment,,"2. PS3-1 is on and has bdaddr 00:13:a9:74:fe:57"
0,,4,0:05.592.405,,,,,,Comment,,"3. PS3-2 is on and has bdaddr 00:23:06:52:e4:e3"
0,,5,0:05.941.930,,,,,,Comment,,"4. Controller previously paired with 00:11:22:33:44:55"
0,,6,0:06.640.981,,,,,,Comment,,"5. Controller powered off with rear button"
0,,7,0:08.738.133,,,,,,Comment,,"6. About to plug controller into PS3-1 (off, wrong pairing)"
0,,285,0:16.314.138,,,,,,Comment,,"7. About to press PS button"
0,,914,0:20.841.170,,,,,,Comment,,"8. Unplugged controller"
0,,915,0:34.123.132,,,,,,Comment,,"9. Controller is now associated with PS3-1"
0,,916,0:41.113.639,,,,,,Comment,,"10. Controller powered off with rear button"
0,,917,0:41.113.639,,,,,,Comment,,"11. About to plug controller into PS3-1 (off, correct pairing)"
0,,1193,1:01.075.280,,,,,,Comment,,"12. Unplugged controller"
0,,1194,1:11.211.515,,,,,,Comment,,"13. Pressed PS button"
0,,1195,1:27.639.205,,,,,,Comment,,"14. Controller is now associated with PS3-1"
0,,1196,1:27.639.205,,,,,,Comment,,"15. About to plug controller into PS3-1 (associated with this PS3)"
0,,2311,1:41.665.351,,,,,,Comment,,"16. Unplugged controller"
0,,2312,1:44.112.029,,,,,,Comment,,"17. Paired and associated controller with PS3-2"
0,,2313,1:57.393.991,,,,,,Comment,,"18. About to plug controller into PS3-1 (associated with different PS3)"
0,,4585,2:26.172.515,,,,,,Comment,,"19. Unplugged controller"
> > The PS3 add-on keyboard, and the PS3 headset both use cable pairing. I
> > have them around, but was unable to get them to pair.
>
> I don't have the hardware so I never searched to see what the
> differences with the Sixaxis are.
I also did some quick captures of pairing the keypad (CECHZK1UC) and
original headset (CECHYA-0075). They are being paired with 00:13:a9:74:fe:57.
http://ps3.jim.sh/sixaxis/dumps/
-jim
^ permalink raw reply
* Re: [PATCH] Emit missing signal when data channel is reconnected.
From: Santiago Carot @ 2011-03-25 13:47 UTC (permalink / raw)
To: Santiago Carot-Nemesio, linux-bluetooth; +Cc: Johan Hedberg
In-Reply-To: <20110325133237.GA6600@jh-x301>
Hi Johan,
See coments below.
2011/3/25 Johan Hedberg <johan.hedberg@gmail.com>:
> Hi,
>
> On Fri, Mar 25, 2011, Santiago Carot-Nemesio wrote:
>> Reconnections of data channels should be indicated to others
>> applications by using the appropriate signal.
>> ---
>> health/hdp.c | 11 ++++++++++-
>> 1 files changed, 10 insertions(+), 1 deletions(-)
>>
>> diff --git a/health/hdp.c b/health/hdp.c
>> index 3c2dce1..b06fe17 100644
>> --- a/health/hdp.c
>> +++ b/health/hdp.c
>> @@ -510,14 +510,23 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
>> }
>>
>> fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
>> - if (fd < 0)
>> + if (fd < 0) {
>> reply = g_dbus_create_error(dc_data->msg,
>> ERROR_INTERFACE ".HealthError",
>> "Cannot get file descriptor");
>> + g_dbus_send_message(dc_data->conn, reply);
>> + return;
>> + }
>>
This is not a memory leak fix, if fd is less than 0, then reconnection
was not succesfully and then we reply with an error response to the
application but we dont not emit the CahnnelConnected signal. (return
statement).
>> reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD, &fd,
>> DBUS_TYPE_INVALID);
>> g_dbus_send_message(dc_data->conn, reply);
At this point we have a valid file descriptor and we can send the signal.
>> +
>> + g_dbus_emit_signal(dc_data->conn,
>> + device_get_path(dc_data->hdp_chann->dev->dev),
>> + HEALTH_DEVICE, "ChannelConnected",
>> + DBUS_TYPE_OBJECT_PATH,
>> + &dc_data->hdp_chann->path, DBUS_TYPE_INVALID);
>> }
>>
>> static void hdp_get_dcpsm_cb(uint16_t dcpsm, gpointer user_data, GError *err)
>
> Looks like this patch is doing two things:
>
> 1. Fix the memory leak/missing g_dbus_send_message call for the return
> value from g_dbus_create_error.
>
> 2. Add sending of the "ChannelConnected" signal.
>
> Could you please split it into two separate patches?
>
> Johan
>
^ permalink raw reply
* Re: [PATCH] Emit missing signal when data channel is reconnected.
From: Johan Hedberg @ 2011-03-25 13:32 UTC (permalink / raw)
To: Santiago Carot-Nemesio; +Cc: linux-bluetooth
In-Reply-To: <1301059143-14614-1-git-send-email-sancane@gmail.com>
Hi,
On Fri, Mar 25, 2011, Santiago Carot-Nemesio wrote:
> Reconnections of data channels should be indicated to others
> applications by using the appropriate signal.
> ---
> health/hdp.c | 11 ++++++++++-
> 1 files changed, 10 insertions(+), 1 deletions(-)
>
> diff --git a/health/hdp.c b/health/hdp.c
> index 3c2dce1..b06fe17 100644
> --- a/health/hdp.c
> +++ b/health/hdp.c
> @@ -510,14 +510,23 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
> }
>
> fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
> - if (fd < 0)
> + if (fd < 0) {
> reply = g_dbus_create_error(dc_data->msg,
> ERROR_INTERFACE ".HealthError",
> "Cannot get file descriptor");
> + g_dbus_send_message(dc_data->conn, reply);
> + return;
> + }
>
> reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD, &fd,
> DBUS_TYPE_INVALID);
> g_dbus_send_message(dc_data->conn, reply);
> +
> + g_dbus_emit_signal(dc_data->conn,
> + device_get_path(dc_data->hdp_chann->dev->dev),
> + HEALTH_DEVICE, "ChannelConnected",
> + DBUS_TYPE_OBJECT_PATH,
> + &dc_data->hdp_chann->path, DBUS_TYPE_INVALID);
> }
>
> static void hdp_get_dcpsm_cb(uint16_t dcpsm, gpointer user_data, GError *err)
Looks like this patch is doing two things:
1. Fix the memory leak/missing g_dbus_send_message call for the return
value from g_dbus_create_error.
2. Add sending of the "ChannelConnected" signal.
Could you please split it into two separate patches?
Johan
^ permalink raw reply
* [PATCH] Bluetooth: Add local Extended Inquiry Response (EIR) support
From: johan.hedberg @ 2011-03-25 13:25 UTC (permalink / raw)
To: linux-bluetooth
From: Johan Hedberg <johan.hedberg@nokia.com>
This patch adds automated creation of the local EIR data based on what
16-bit UUIDs are registered and what the device name is. This should
cover the majority use cases, however things like 32/128-bit UUIDs, TX
power and Device ID will need to be added later to be on par with what
bluetoothd is capable of doing (without the Management interface).
Signed-off-by: Johan Hedberg <johan.hedberg@nokia.com>
---
include/net/bluetooth/hci.h | 8 ++
include/net/bluetooth/hci_core.h | 1 +
net/bluetooth/mgmt.c | 164 ++++++++++++++++++++++++++++++++++++++
3 files changed, 173 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index b989a8c..6846ec0 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -614,6 +614,14 @@ struct hci_cp_host_buffer_size {
#define HCI_OP_WRITE_INQUIRY_MODE 0x0c45
+#define HCI_MAX_EIR_LENGTH 240
+
+#define HCI_OP_WRITE_EIR 0x0c52
+struct hci_cp_write_eir {
+ uint8_t fec;
+ uint8_t data[HCI_MAX_EIR_LENGTH];
+} __packed;
+
#define HCI_OP_READ_SSP_MODE 0x0c55
struct hci_rp_read_ssp_mode {
__u8 status;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 87bff51..3b2f09d 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -102,6 +102,7 @@ struct hci_dev {
__u8 dev_type;
bdaddr_t bdaddr;
__u8 dev_name[HCI_MAX_NAME_LENGTH];
+ __u8 eir[HCI_MAX_EIR_LENGTH];
__u8 dev_class[3];
__u8 major_class;
__u8 minor_class;
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a42dc8c..d8cbf4d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -544,6 +544,151 @@ failed:
return err;
}
+#define EIR_FLAGS 0x01 /* flags */
+#define EIR_UUID16_SOME 0x02 /* 16-bit UUID, more available */
+#define EIR_UUID16_ALL 0x03 /* 16-bit UUID, all listed */
+#define EIR_UUID32_SOME 0x04 /* 32-bit UUID, more available */
+#define EIR_UUID32_ALL 0x05 /* 32-bit UUID, all listed */
+#define EIR_UUID128_SOME 0x06 /* 128-bit UUID, more available */
+#define EIR_UUID128_ALL 0x07 /* 128-bit UUID, all listed */
+#define EIR_NAME_SHORT 0x08 /* shortened local name */
+#define EIR_NAME_COMPLETE 0x09 /* complete local name */
+#define EIR_TX_POWER 0x0A /* transmit power level */
+#define EIR_DEVICE_ID 0x10 /* device ID */
+
+#define PNP_INFO_SVCLASS_ID 0x1200
+
+static u8 bluetooth_base_uuid[] = {
+ 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
+ 0x80, 0x00, 0x00, 0x80, 0x5F, 0x9B, 0x34, 0xFB,
+};
+
+static u16 get_uuid16(u8 *uuid128)
+{
+ u8 *b = bluetooth_base_uuid;
+ u32 val;
+ int i;
+
+ for (i = 4; i < 16; i++) {
+ if (b[i] != uuid128[i])
+ return 0;
+ }
+
+ memcpy(&val, uuid128, 4);
+
+ val = be32_to_cpu(val);
+ if (val > 0xffff)
+ return 0;
+
+ return (u16) val;
+}
+
+static void create_eir(struct hci_dev *hdev, u8 *data)
+{
+ u8 *ptr = data;
+ u16 eir_len = 0;
+ u16 uuid16_list[HCI_MAX_EIR_LENGTH / sizeof(u16)];
+ int i, truncated = 0;
+ struct list_head *p;
+ size_t name_len;
+
+ name_len = strlen(hdev->dev_name);
+
+ if (name_len > 0) {
+ /* EIR Data type */
+ if (name_len > 48) {
+ name_len = 48;
+ ptr[1] = EIR_NAME_SHORT;
+ } else
+ ptr[1] = EIR_NAME_COMPLETE;
+
+ /* EIR Data length */
+ ptr[0] = name_len + 1;
+
+ memcpy(ptr + 2, hdev->dev_name, name_len);
+
+ eir_len += (name_len + 2);
+ ptr += (name_len + 2);
+ }
+
+ memset(uuid16_list, 0, sizeof(uuid16_list));
+
+ /* Group all UUID16 types */
+ list_for_each(p, &hdev->uuids) {
+ struct bt_uuid *uuid = list_entry(p, struct bt_uuid, list);
+ u16 uuid16;
+
+ uuid16 = get_uuid16(uuid->uuid);
+ if (uuid16 == 0)
+ return;
+
+ if (uuid16 < 0x1100)
+ continue;
+
+ if (uuid16 == PNP_INFO_SVCLASS_ID)
+ continue;
+
+ /* Stop if not enough space to put next UUID */
+ if (eir_len + 2 + sizeof(u16) > HCI_MAX_EIR_LENGTH) {
+ truncated = 1;
+ break;
+ }
+
+ /* Check for duplicates */
+ for (i = 0; uuid16_list[i] != 0; i++)
+ if (uuid16_list[i] == uuid16)
+ break;
+
+ if (uuid16_list[i] == 0) {
+ uuid16_list[i] = uuid16;
+ eir_len += sizeof(u16);
+ }
+ }
+
+ if (uuid16_list[0] != 0) {
+ u8 *length = ptr;
+
+ /* EIR Data type */
+ ptr[1] = truncated ? EIR_UUID16_SOME : EIR_UUID16_ALL;
+
+ ptr += 2;
+ eir_len += 2;
+
+ for (i = 0; uuid16_list[i] != 0; i++) {
+ *ptr++ = (uuid16_list[i] & 0x00ff);
+ *ptr++ = (uuid16_list[i] & 0xff00) >> 8;
+ }
+
+ /* EIR Data length */
+ *length = (i * sizeof(u16)) + 1;
+ }
+}
+
+static int update_eir(struct hci_dev *hdev)
+{
+ struct hci_cp_write_eir cp;
+
+ if (!(hdev->features[6] & LMP_EXT_INQ))
+ return 0;
+
+ if (hdev->ssp_mode == 0)
+ return 0;
+
+ if (test_bit(HCI_SERVICE_CACHE, &hdev->flags))
+ return 0;
+
+ memset(&cp, 0, sizeof(cp));
+
+ create_eir(hdev, cp.data);
+
+ if (memcmp(cp.data, hdev->eir, sizeof(cp.data)) == 0)
+ return 0;
+
+ memcpy(hdev->eir, cp.data, sizeof(cp.data));
+
+ return hci_send_cmd(hdev, HCI_OP_WRITE_EIR, sizeof(cp), &cp);
+}
+
static u8 get_service_classes(struct hci_dev *hdev)
{
struct list_head *p;
@@ -612,6 +757,10 @@ static int add_uuid(struct sock *sk, u16 index, unsigned char *data, u16 len)
if (err < 0)
goto failed;
+ err = update_eir(hdev);
+ if (err < 0)
+ goto failed;
+
err = cmd_complete(sk, index, MGMT_OP_ADD_UUID, NULL, 0);
failed:
@@ -668,6 +817,10 @@ static int remove_uuid(struct sock *sk, u16 index, unsigned char *data, u16 len)
if (err < 0)
goto unlock;
+ err = update_eir(hdev);
+ if (err < 0)
+ goto unlock;
+
err = cmd_complete(sk, index, MGMT_OP_REMOVE_UUID, NULL, 0);
unlock:
@@ -737,6 +890,8 @@ static int set_service_cache(struct sock *sk, u16 index, unsigned char *data,
} else {
clear_bit(HCI_SERVICE_CACHE, &hdev->flags);
err = update_class(hdev);
+ if (err == 0)
+ err = update_eir(hdev);
}
if (err == 0)
@@ -1822,6 +1977,7 @@ int mgmt_auth_failed(u16 index, bdaddr_t *bdaddr, u8 status)
int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status)
{
struct pending_cmd *cmd;
+ struct hci_dev *hdev;
struct mgmt_cp_set_local_name ev;
int err;
@@ -1837,6 +1993,14 @@ int mgmt_set_local_name_complete(u16 index, u8 *name, u8 status)
goto failed;
}
+ hdev = hci_dev_get(index);
+ if (hdev) {
+ hci_dev_lock_bh(hdev);
+ update_eir(hdev);
+ hci_dev_unlock_bh(hdev);
+ hci_dev_put(hdev);
+ }
+
err = cmd_complete(cmd->sk, index, MGMT_OP_SET_LOCAL_NAME, &ev,
sizeof(ev));
if (err < 0)
--
1.7.4.1
^ permalink raw reply related
* [PATCH] Emit missing signal when data channel is reconnected.
From: Santiago Carot-Nemesio @ 2011-03-25 13:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
Reconnections of data channels should be indicated to others
applications by using the appropriate signal.
---
health/hdp.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/health/hdp.c b/health/hdp.c
index 3c2dce1..b06fe17 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -510,14 +510,23 @@ static void hdp_mdl_reconn_cb(struct mcap_mdl *mdl, GError *err, gpointer data)
}
fd = mcap_mdl_get_fd(dc_data->hdp_chann->mdl);
- if (fd < 0)
+ if (fd < 0) {
reply = g_dbus_create_error(dc_data->msg,
ERROR_INTERFACE ".HealthError",
"Cannot get file descriptor");
+ g_dbus_send_message(dc_data->conn, reply);
+ return;
+ }
reply = g_dbus_create_reply(dc_data->msg, DBUS_TYPE_UNIX_FD, &fd,
DBUS_TYPE_INVALID);
g_dbus_send_message(dc_data->conn, reply);
+
+ g_dbus_emit_signal(dc_data->conn,
+ device_get_path(dc_data->hdp_chann->dev->dev),
+ HEALTH_DEVICE, "ChannelConnected",
+ DBUS_TYPE_OBJECT_PATH,
+ &dc_data->hdp_chann->path, DBUS_TYPE_INVALID);
}
static void hdp_get_dcpsm_cb(uint16_t dcpsm, gpointer user_data, GError *err)
--
1.7.4.1
^ permalink raw reply related
* Re: [RFCv1 3/3] Bluetooth: delete hanging L2CAP channel
From: Gustavo F. Padovan @ 2011-03-25 12:35 UTC (permalink / raw)
To: Andrei Emeltchenko; +Cc: linux-bluetooth
In-Reply-To: <AANLkTim2_owHJ9NT4B7u484D1itkuEmGUzdp1wxgaZTi@mail.gmail.com>
Hi Andrei,
* Andrei Emeltchenko <andrei.emeltchenko.news@gmail.com> [2011-03-25 10:57:52 +0200]:
> Hi Gustavo,
>
> On Thu, Mar 24, 2011 at 9:12 PM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> > Hi Andrei,
> >
> > * Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2011-03-24 17:16:08 +0200]:
> >
> >> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> >>
> >> Sometimes L2CAP connection remains hanging. Make sure that
> >> L2CAP channel is deleted.
> >>
> >> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
> >> ---
> >> net/bluetooth/l2cap_sock.c | 5 +++--
> >> 1 files changed, 3 insertions(+), 2 deletions(-)
> >
> > This one is applied. Thanks.
>
> Where it is applied? Cannot find it here:
I applied it to bluetooth-2.6 as it is a bug fix.
--
Gustavo F. Padovan
http://profusion.mobi
^ permalink raw reply
* Re: [RFCv1 2/3] Bluetooth: remove duplicated code
From: Andrei Emeltchenko @ 2011-03-25 9:35 UTC (permalink / raw)
To: Emeltchenko Andrei, linux-bluetooth; +Cc: Gustavo F. Padovan
In-Reply-To: <20110324185759.GB2236@joana>
Hi Gustavo,
On Thu, Mar 24, 2011 at 8:57 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> Hi Andrei,
>
> * Emeltchenko Andrei <Andrei.Emeltchenko.news@gmail.com> [2011-03-24 17:16:07 +0200]:
>
>> From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>>
>> info_timer takes care about removed code
>>
>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> ---
>> net/bluetooth/l2cap_core.c | 12 +++---------
>> 1 files changed, 3 insertions(+), 9 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
>> index fd58b8f..4255f00 100644
>> --- a/net/bluetooth/l2cap_core.c
>> +++ b/net/bluetooth/l2cap_core.c
>> @@ -2472,16 +2472,10 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
>> return 0;
>> }
>>
>> - del_timer(&conn->info_timer);
>> -
>> - if (result != L2CAP_IR_SUCCESS) {
>> - conn->info_state |= L2CAP_INFO_FEAT_MASK_REQ_DONE;
>> - conn->info_ident = 0;
>> -
>> - l2cap_conn_start(conn);
>
> Are you sure? Does remove this code won't create a delay to call
> l2cap_conn_start()?
Yes, but the difference is only with delay. I believe that this way it
will be cleaner.
Do you think that it could cause problems?
Regards,
Andrei
^ permalink raw reply
* [PATCH] Bluetooth: check L2CAP info_rsp ident and state
From: Emeltchenko Andrei @ 2011-03-25 9:31 UTC (permalink / raw)
To: linux-bluetooth
From: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
Information requests/responses are unbound to L2CAP channel. Patch
fixes issue arising when two devices connects at the same time to
each other. This way we do not process out of the context messages.
We are safe dropping info_rsp since info_timer is left running.
Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
---
net/bluetooth/l2cap_core.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index b5a1ce0..1426c03 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -2460,6 +2460,11 @@ static inline int l2cap_information_rsp(struct l2cap_conn *conn, struct l2cap_cm
BT_DBG("type 0x%4.4x result 0x%2.2x", type, result);
+ /* L2CAP Info req/rsp are unbound to channels, add extra checks */
+ if (cmd->ident != conn->info_ident ||
+ conn->info_state & L2CAP_INFO_FEAT_MASK_REQ_DONE)
+ return 0;
+
del_timer(&conn->info_timer);
if (result != L2CAP_IR_SUCCESS) {
--
1.7.1
^ permalink raw reply related
* Re: [PATCH v2] Add sap_disconnect_ind interface for sap-sim drivers
From: Johan Hedberg @ 2011-03-25 9:17 UTC (permalink / raw)
To: Waldemar Rymarkiewicz; +Cc: linux-bluetooth
In-Reply-To: <1300983402-2970-1-git-send-email-waldemar.rymarkiewicz@tieto.com>
Hi Waldek,
On Thu, Mar 24, 2011, Waldemar Rymarkiewicz wrote:
> The sap_disconnect_ind() let's the sim driver to indicate
> immediate disconnection.
>
> Add support of immediate disconnection in sap-dummy driver
> as well as a card status change in order to pass all PTS tests.
> ---
> sap/sap-dummy.c | 31 ++++++++++++++++++++++++-------
> sap/sap.h | 1 +
> sap/server.c | 7 +++++++
> 3 files changed, 32 insertions(+), 7 deletions(-)
Are you planning to have some additional Disconnect* method in the
future since you didn't reuse the existing "Disconnect" method name for
this? The patch is also missing the corresponding update to sap-api.txt.
Johan
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox