From: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
In case of multiple session being active the code was registering one
PDU hanlder per AVRCP session and given the session pointer as user data
causing the following:
24 bytes in 1 blocks are definitely lost in loss record 370 of 893
at 0x4A0884D: malloc (vg_replace_malloc.c:263)
by 0x4C803FE: g_malloc (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x12EE9D: avctp_register_browsing_pdu_handler (avctp.c:1259)
by 0x12FD7B: state_changed (avrcp.c:1402)
by 0x12D391: avctp_set_state (avctp.c:403)
by 0x12E6B4: avctp_confirm_cb (avctp.c:871)
by 0x1606A3: server_cb (btio.c:254)
by 0x4C7A824: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4C7AB57: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4C7AF51: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x120EA1: main (main.c:551)
To fix this the PDU handlers are now per AVCTP session/channel so each
AVRCP session can register its own handler and pass itself as user
data.
---
audio/avctp.c | 180 ++++++++++++++++++++++++++++++++++++----------------------
audio/avctp.h | 10 ++--
audio/avrcp.c | 56 +++++++++---------
audio/avrcp.h | 1 +
4 files changed, 148 insertions(+), 99 deletions(-)
diff --git a/audio/avctp.c b/audio/avctp.c
index aa9a1ca..228d5b7 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -137,6 +137,7 @@ struct avctp_channel {
uint16_t imtu;
uint16_t omtu;
uint8_t *buffer;
+ GSList *handlers;
};
struct avctp {
@@ -148,6 +149,9 @@ struct avctp {
int uinput;
guint auth_id;
+ unsigned int passthrough_id;
+ unsigned int unit_id;
+ unsigned int subunit_id;
struct avctp_channel *control;
struct avctp_channel *browsing;
@@ -186,9 +190,7 @@ static struct {
static GSList *callbacks = NULL;
static GSList *servers = NULL;
-static GSList *control_handlers = NULL;
static uint8_t id = 0;
-static struct avctp_browsing_pdu_handler *browsing_handler = NULL;
static void auth_cb(DBusError *derr, void *user_data);
@@ -346,6 +348,7 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
g_source_remove(chan->watch);
g_free(chan->buffer);
+ g_slist_free_full(chan->handlers, g_free);
g_free(chan);
}
@@ -454,6 +457,7 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
uint8_t *operands;
struct avctp_header *avctp;
int sock, ret, packet_size, operand_count;
+ struct avctp_browsing_pdu_handler *handler;
if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
goto failed;
@@ -480,10 +484,18 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
packet_size = AVCTP_HEADER_LENGTH;
avctp->cr = AVCTP_RESPONSE;
- packet_size += browsing_handler->cb(session, avctp->transaction,
+ handler = g_slist_nth_data(browsing->handlers, 0);
+ if (handler == NULL) {
+ DBG("handler not found");
+ packet_size += avrcp_browsing_general_reject(operands);
+ goto send;
+ }
+
+ packet_size += handler->cb(session, avctp->transaction,
operands, operand_count,
- browsing_handler->user_data);
+ handler->user_data);
+send:
if (packet_size != 0) {
ret = write(sock, buf, packet_size);
if (ret != packet_size)
@@ -570,7 +582,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
goto done;
}
- handler = find_handler(control_handlers, avc->opcode);
+ handler = find_handler(control->handlers, avc->opcode);
if (!handler) {
DBG("handler not found for 0x%02x", avc->opcode);
packet_size += avrcp_handle_vendor_reject(&code, operands);
@@ -774,6 +786,19 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
(GIOFunc) session_cb, session);
+ session->passthrough_id = avctp_register_pdu_handler(session,
+ AVC_OP_PASSTHROUGH,
+ handle_panel_passthrough,
+ NULL);
+ session->unit_id = avctp_register_pdu_handler(session,
+ AVC_OP_UNITINFO,
+ handle_unit_info,
+ NULL);
+ session->subunit_id = avctp_register_pdu_handler(session,
+ AVC_OP_SUBUNITINFO,
+ handle_subunit_info,
+ NULL);
+
init_uinput(session);
avctp_set_state(session, AVCTP_STATE_CONNECTED);
@@ -896,12 +921,6 @@ static void avctp_browsing_confirm(struct avctp *session, GIOChannel *chan,
return;
}
- if (browsing_handler == NULL) {
- error("Browsing: Handler not registered");
- g_io_channel_shutdown(chan, TRUE, NULL);
- return;
- }
-
if (bt_io_accept(chan, avctp_connect_browsing_cb, session, NULL,
&err))
return;
@@ -994,10 +1013,6 @@ static GIOChannel *avctp_server_socket(const bdaddr_t *src, gboolean master,
return io;
}
-static unsigned int passthrough_id = 0;
-static unsigned int unit_id = 0;
-static unsigned int subunit_id = 0;
-
int avctp_register(const bdaddr_t *src, gboolean master)
{
struct avctp_server *server;
@@ -1026,18 +1041,6 @@ int avctp_register(const bdaddr_t *src, gboolean master)
servers = g_slist_append(servers, server);
- if (!passthrough_id)
- passthrough_id = avctp_register_pdu_handler(AVC_OP_PASSTHROUGH,
- handle_panel_passthrough, NULL);
-
- if (!unit_id)
- unit_id = avctp_register_pdu_handler(AVC_OP_UNITINFO, handle_unit_info,
- NULL);
-
- if (!subunit_id)
- subunit_id = avctp_register_pdu_handler(AVC_OP_SUBUNITINFO,
- handle_subunit_info, NULL);
-
return 0;
}
@@ -1061,24 +1064,6 @@ void avctp_unregister(const bdaddr_t *src)
g_io_channel_shutdown(server->control_io, TRUE, NULL);
g_io_channel_unref(server->control_io);
g_free(server);
-
- if (servers)
- return;
-
- if (passthrough_id) {
- avctp_unregister_pdu_handler(passthrough_id);
- passthrough_id = 0;
- }
-
- if (unit_id) {
- avctp_unregister_pdu_handler(unit_id);
- passthrough_id = 0;
- }
-
- if (subunit_id) {
- avctp_unregister_pdu_handler(subunit_id);
- subunit_id = 0;
- }
}
int avctp_send_passthrough(struct avctp *session, uint8_t op)
@@ -1230,13 +1215,18 @@ gboolean avctp_remove_state_cb(unsigned int id)
return FALSE;
}
-unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
- void *user_data)
+unsigned int avctp_register_pdu_handler(struct avctp *session, uint8_t opcode,
+ avctp_control_pdu_cb cb,
+ void *user_data)
{
+ struct avctp_channel *control = session->control;
struct avctp_pdu_handler *handler;
static unsigned int id = 0;
- handler = find_handler(control_handlers, opcode);
+ if (control == NULL)
+ return 0;
+
+ handler = find_handler(control->handlers, opcode);
if (handler)
return 0;
@@ -1246,36 +1236,63 @@ unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
handler->user_data = user_data;
handler->id = ++id;
- control_handlers = g_slist_append(control_handlers, handler);
+ control->handlers = g_slist_append(control->handlers, handler);
return handler->id;
}
-unsigned int avctp_register_browsing_pdu_handler(avctp_browsing_pdu_cb cb,
- void *user_data)
+unsigned int avctp_register_browsing_pdu_handler(struct avctp *session,
+ avctp_browsing_pdu_cb cb,
+ void *user_data)
{
- unsigned int id = 0;
+ struct avctp_channel *browsing = session->browsing;
+ struct avctp_browsing_pdu_handler *handler;
+ static unsigned int id = 0;
+
+ if (browsing == NULL)
+ return 0;
+
+ if (browsing->handlers != NULL)
+ return 0;
+
+ handler = g_new(struct avctp_browsing_pdu_handler, 1);
+ handler->cb = cb;
+ handler->user_data = user_data;
+ handler->id = ++id;
- browsing_handler = g_new(struct avctp_browsing_pdu_handler, 1);
- browsing_handler->cb = cb;
- browsing_handler->user_data = user_data;
- browsing_handler->id = ++id;
+ browsing->handlers = g_slist_append(browsing->handlers, handler);
- return browsing_handler->id;
+ return handler->id;
}
gboolean avctp_unregister_pdu_handler(unsigned int id)
{
GSList *l;
- for (l = control_handlers; l != NULL; l = l->next) {
- struct avctp_pdu_handler *handler = l->data;
+ for (l = servers; l; l = l->next) {
+ struct avctp_server *server = l->data;
+ GSList *s;
- if (handler->id == id) {
- control_handlers = g_slist_remove(control_handlers,
- handler);
- g_free(handler);
- return TRUE;
+ for (s = server->sessions; s; s = s->next) {
+ struct avctp *session = s->data;
+ struct avctp_channel *control = session->control;
+ GSList *h;
+
+ if (control == NULL)
+ continue;
+
+ for (h = control->handlers; h; h = h->next) {
+ struct avctp_pdu_handler *handler = h->data;
+
+ if (handler->id != id)
+ continue;
+
+ control->handlers = g_slist_remove(
+ control->handlers,
+ handler);
+ g_free(handler);
+ return TRUE;
+ }
}
}
@@ -1284,12 +1301,37 @@ gboolean avctp_unregister_pdu_handler(unsigned int id)
gboolean avctp_unregister_browsing_pdu_handler(unsigned int id)
{
- if (browsing_handler->id != id)
- return FALSE;
+ GSList *l;
- g_free(browsing_handler);
- browsing_handler = NULL;
- return TRUE;
+ for (l = servers; l; l = l->next) {
+ struct avctp_server *server = l->data;
+ GSList *s;
+
+ for (s = server->sessions; s; s = s->next) {
+ struct avctp *session = l->data;
+ struct avctp_channel *browsing = session->browsing;
+ GSList *h;
+
+ if (browsing == NULL)
+ continue;
+
+ for (h = browsing->handlers; h; h = h->next) {
+ struct avctp_browsing_pdu_handler *handler =
+ h->data;
+
+ if (handler->id != id)
+ continue;
+
+ browsing->handlers = g_slist_remove(
+ browsing->handlers,
+ handler);
+ g_free(handler);
+ return TRUE;
+ }
+ }
+ }
+
+ return FALSE;
}
struct avctp *avctp_connect(const bdaddr_t *src, const bdaddr_t *dst)
diff --git a/audio/avctp.h b/audio/avctp.h
index 6d39a20..41cf6c5 100644
--- a/audio/avctp.h
+++ b/audio/avctp.h
@@ -98,12 +98,14 @@ struct avctp *avctp_get(const bdaddr_t *src, const bdaddr_t *dst);
int avctp_connect_browsing(struct avctp *session);
void avctp_disconnect(struct avctp *session);
-unsigned int avctp_register_pdu_handler(uint8_t opcode, avctp_control_pdu_cb cb,
- void *user_data);
+unsigned int avctp_register_pdu_handler(struct avctp *session, uint8_t opcode,
+ avctp_control_pdu_cb cb,
+ void *user_data);
gboolean avctp_unregister_pdu_handler(unsigned int id);
-unsigned int avctp_register_browsing_pdu_handler(avctp_browsing_pdu_cb cb,
- void *user_data);
+unsigned int avctp_register_browsing_pdu_handler(struct avctp *session,
+ avctp_browsing_pdu_cb cb,
+ void *user_data);
gboolean avctp_unregister_browsing_pdu_handler(unsigned int id);
int avctp_send_passthrough(struct avctp *session, uint8_t op);
diff --git a/audio/avrcp.c b/audio/avrcp.c
index 1ed6a24..f31372b 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -184,8 +184,8 @@ struct avrcp {
uint16_t version;
int features;
- unsigned int control_handler;
- unsigned int browsing_handler;
+ unsigned int control_id;
+ unsigned int browsing_id;
uint16_t registered_events;
uint8_t transaction;
uint8_t transaction_events[AVRCP_EVENT_LAST + 1];
@@ -1228,6 +1228,19 @@ static struct browsing_pdu_handler {
{ },
};
+size_t avrcp_browsing_general_reject(uint8_t *operands)
+{
+ struct avrcp_browsing_header *pdu = (void *) operands;
+ uint8_t status;
+
+ pdu->pdu_id = AVRCP_GENERAL_REJECT;
+ status = AVRCP_STATUS_INVALID_COMMAND;
+
+ pdu->param_len = htons(sizeof(status));
+ memcpy(pdu->params, &status, (sizeof(status)));
+ return AVRCP_BROWSING_HEADER_LENGTH + sizeof(status);
+}
+
static size_t handle_browsing_pdu(struct avctp *conn,
uint8_t transaction, uint8_t *operands,
size_t operand_count, void *user_data)
@@ -1235,7 +1248,6 @@ static size_t handle_browsing_pdu(struct avctp *conn,
struct avrcp *session = user_data;
struct browsing_pdu_handler *handler;
struct avrcp_browsing_header *pdu = (void *) operands;
- uint8_t status;
DBG("AVRCP Browsing PDU 0x%02X, len 0x%04X", pdu->pdu_id,
pdu->param_len);
@@ -1245,20 +1257,12 @@ static size_t handle_browsing_pdu(struct avctp *conn,
break;
}
- if (handler == NULL || handler->func == NULL) {
- pdu->pdu_id = AVRCP_GENERAL_REJECT;
- status = AVRCP_STATUS_INVALID_COMMAND;
- goto err;
- }
+ if (handler == NULL || handler->func == NULL)
+ return avrcp_browsing_general_reject(operands);
session->transaction = transaction;
handler->func(session, pdu, transaction);
return AVRCP_BROWSING_HEADER_LENGTH + ntohs(pdu->param_len);
-
-err:
- pdu->param_len = htons(sizeof(status));
- memcpy(pdu->params, &status, (sizeof(status)));
- return AVRCP_BROWSING_HEADER_LENGTH + sizeof(status);
}
size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands)
@@ -1370,12 +1374,12 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
server->sessions = g_slist_remove(server->sessions, session);
- if (session->control_handler)
- avctp_unregister_pdu_handler(session->control_handler);
+ if (session->control_id > 0)
+ avctp_unregister_pdu_handler(session->control_id);
- if (session->browsing_handler)
+ if (session->browsing_id > 0)
avctp_unregister_browsing_pdu_handler(
- session->browsing_handler);
+ session->browsing_id);
if (session->player != NULL)
session->player->sessions = g_slist_remove(
@@ -1394,15 +1398,6 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
session->dev = dev;
session->player = g_slist_nth_data(server->players, 0);
- session->control_handler = avctp_register_pdu_handler(
- AVC_OP_VENDORDEP,
- handle_vendordep_pdu,
- session);
- session->browsing_handler =
- avctp_register_browsing_pdu_handler(
- handle_browsing_pdu,
- session);
-
server->sessions = g_slist_append(server->sessions, session);
rec = btd_device_get_record(dev->btd_dev, AVRCP_TARGET_UUID);
@@ -1429,6 +1424,15 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
if (session->features & AVRCP_FEATURE_BROWSING)
avctp_connect_browsing(session->conn);
}
+
+ session->control_id = avctp_register_pdu_handler(session->conn,
+ AVC_OP_VENDORDEP,
+ handle_vendordep_pdu,
+ session);
+ session->browsing_id = avctp_register_browsing_pdu_handler(
+ session->conn,
+ handle_browsing_pdu,
+ session);
default:
return;
}
diff --git a/audio/avrcp.h b/audio/avrcp.h
index 42d902b..6c651dd 100644
--- a/audio/avrcp.h
+++ b/audio/avrcp.h
@@ -105,3 +105,4 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data);
size_t avrcp_handle_vendor_reject(uint8_t *code, uint8_t *operands);
+size_t avrcp_browsing_general_reject(uint8_t *operands);
--
1.7.11.4
--
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
