2012-10-01 11:37:45

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 1/7] audio: Fix not freeing gateway agent data on exit

From: Luiz Augusto von Dentz <[email protected]>

434 (168 direct, 266 indirect) bytes in 7 blocks are definitely lost in loss record 322 of 338
at 0x4A06F18: calloc (vg_replace_malloc.c:566)
by 0x4C802C6: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x126C0C: register_agent (gateway.c:673)
by 0x122960: process_message.isra.0 (object.c:197)
by 0x4F70684: ??? (in /usr/lib64/libdbus-1.so.3.5.6)
by 0x4F6290C: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.5.6)
by 0x120FA7: message_dispatch (mainloop.c:76)
by 0x4C7B22A: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4C7A694: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4C7A9C7: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4C7ADC1: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x120671: main (main.c:551)
---
audio/gateway.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/audio/gateway.c b/audio/gateway.c
index 45b25a1..3e606c9 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -741,6 +741,9 @@ static void path_unregister(void *data)

gateway_close(dev);

+ if (dev->gateway->agent)
+ agent_free(dev->gateway->agent);
+
g_free(dev->gateway);
dev->gateway = NULL;
}
--
1.7.11.4



2012-10-01 13:47:33

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/7] audio: Fix not freeing gateway agent data on exit

Hi Luiz,

On Mon, Oct 01, 2012, Luiz Augusto von Dentz wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> 434 (168 direct, 266 indirect) bytes in 7 blocks are definitely lost in loss record 322 of 338
> at 0x4A06F18: calloc (vg_replace_malloc.c:566)
> by 0x4C802C6: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.3200.4)
> by 0x126C0C: register_agent (gateway.c:673)
> by 0x122960: process_message.isra.0 (object.c:197)
> by 0x4F70684: ??? (in /usr/lib64/libdbus-1.so.3.5.6)
> by 0x4F6290C: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.5.6)
> by 0x120FA7: message_dispatch (mainloop.c:76)
> by 0x4C7B22A: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
> by 0x4C7A694: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3200.4)
> by 0x4C7A9C7: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
> by 0x4C7ADC1: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3200.4)
> by 0x120671: main (main.c:551)
> ---
> audio/gateway.c | 3 +++
> 1 file changed, 3 insertions(+)

All patches in this set have been applied. Thanks.

Johan

2012-10-01 12:31:50

by Mikel Astiz

[permalink] [raw]
Subject: Re: [PATCH BlueZ 1/7] audio: Fix not freeing gateway agent data on exit

Hi Luiz,

On Mon, Oct 1, 2012 at 1:37 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> 434 (168 direct, 266 indirect) bytes in 7 blocks are definitely lost in loss record 322 of 338
> at 0x4A06F18: calloc (vg_replace_malloc.c:566)
> by 0x4C802C6: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.3200.4)
> by 0x126C0C: register_agent (gateway.c:673)
> by 0x122960: process_message.isra.0 (object.c:197)
> by 0x4F70684: ??? (in /usr/lib64/libdbus-1.so.3.5.6)
> by 0x4F6290C: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.5.6)
> by 0x120FA7: message_dispatch (mainloop.c:76)
> by 0x4C7B22A: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
> by 0x4C7A694: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3200.4)
> by 0x4C7A9C7: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
> by 0x4C7ADC1: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3200.4)
> by 0x120671: main (main.c:551)
> ---
> audio/gateway.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/audio/gateway.c b/audio/gateway.c
> index 45b25a1..3e606c9 100644
> --- a/audio/gateway.c
> +++ b/audio/gateway.c
> @@ -741,6 +741,9 @@ static void path_unregister(void *data)
>
> gateway_close(dev);
>
> + if (dev->gateway->agent)
> + agent_free(dev->gateway->agent);
> +
> g_free(dev->gateway);
> dev->gateway = NULL;
> }
> --

I also had this in my pipeline so ack from my side.

Cheers,
Mikel

2012-10-01 11:37:49

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 5/7] AVCTP: Allocate memory to hold incoming/outgoing PDUs

From: Luiz Augusto von Dentz <[email protected]>

This makes possible to the handler to respond asyncronous as the memory
remains valid after it returns.

In addition to that it uses the MTU to calculate the buffer size
necessary.
---
audio/avctp.c | 31 ++++++++++++++++++++++---------
1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/audio/avctp.c b/audio/avctp.c
index dfc5601..baf5da0 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -134,7 +134,9 @@ struct avctp_rsp_handler {
struct avctp_channel {
GIOChannel *io;
guint watch;
- uint16_t mtu;
+ uint16_t imtu;
+ uint16_t omtu;
+ uint8_t *buffer;
};

struct avctp {
@@ -341,6 +343,7 @@ static void avctp_channel_destroy(struct avctp_channel *chan)
if (chan->watch)
g_source_remove(chan->watch);

+ g_free(chan->buffer);
g_free(chan);
}

@@ -447,7 +450,9 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
gpointer data)
{
struct avctp *session = data;
- uint8_t buf[1024], *operands;
+ struct avctp_channel *browsing = session->browsing;
+ uint8_t *buf = browsing->buffer;
+ uint8_t *operands;
struct avctp_header *avctp;
int sock, ret, packet_size, operand_count;

@@ -456,7 +461,7 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,

sock = g_io_channel_unix_get_fd(chan);

- ret = read(sock, buf, sizeof(buf));
+ ret = read(sock, buf, sizeof(browsing->imtu));
if (ret <= 0)
goto failed;

@@ -497,7 +502,9 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
gpointer data)
{
struct avctp *session = data;
- uint8_t buf[1024], *operands, code, subunit;
+ struct avctp_channel *control = session->control;
+ uint8_t *buf = control->buffer;
+ uint8_t *operands, code, subunit;
struct avctp_header *avctp;
struct avc_header *avc;
int ret, packet_size, operand_count, sock;
@@ -508,7 +515,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,

sock = g_io_channel_unix_get_fd(chan);

- ret = read(sock, buf, sizeof(buf));
+ ret = read(sock, buf, control->imtu);
if (ret <= 0)
goto failed;

@@ -689,7 +696,7 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
{
struct avctp *session = data;
char address[18];
- uint16_t imtu;
+ uint16_t imtu, omtu;
GError *gerr = NULL;

if (err) {
@@ -700,6 +707,7 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
bt_io_get(chan, &gerr,
BT_IO_OPT_DEST, &address,
BT_IO_OPT_IMTU, &imtu,
+ BT_IO_OPT_OMTU, &omtu,
BT_IO_OPT_INVALID);
if (gerr) {
error("%s", gerr->message);
@@ -714,7 +722,9 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
if (session->browsing == NULL)
session->browsing = avctp_channel_create(chan);

- session->browsing->mtu = imtu;
+ session->browsing->imtu = imtu;
+ session->browsing->omtu = omtu;
+ session->browsing->buffer = g_malloc0(MAX(imtu, omtu));
session->browsing->watch = g_io_add_watch(session->browsing->io,
G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
(GIOFunc) session_browsing_cb, session);
@@ -732,7 +742,7 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
{
struct avctp *session = data;
char address[18];
- uint16_t imtu;
+ uint16_t imtu, omtu;
GError *gerr = NULL;

if (err) {
@@ -744,6 +754,7 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
bt_io_get(chan, &gerr,
BT_IO_OPT_DEST, &address,
BT_IO_OPT_IMTU, &imtu,
+ BT_IO_OPT_IMTU, &omtu,
BT_IO_OPT_INVALID);
if (gerr) {
avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
@@ -757,7 +768,9 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
if (session->control == NULL)
session->control = avctp_channel_create(chan);

- session->control->mtu = imtu;
+ session->control->imtu = imtu;
+ session->control->omtu = omtu;
+ session->control->buffer = g_malloc0(MAX(imtu, omtu));
session->control->watch = g_io_add_watch(session->control->io,
G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
(GIOFunc) session_cb, session);
--
1.7.11.4


2012-10-01 11:37:51

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 7/7] AVRCP: Don't respond with errors when no player is registered

From: Luiz Augusto von Dentz <[email protected]>

Some devices w.g. Sony MW600 will stop using certain commands if an
error happen, so the code now just fake a player and once a real
player is registered it takes place of the fake one.
---
audio/avrcp.c | 77 ++++++++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 63 insertions(+), 14 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 21d105a..6b426f5 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -477,6 +477,17 @@ void avrcp_player_event(struct avrcp_player *player, uint8_t id, void *data)
return;
}

+static void *player_get_metadata(struct avrcp_player *player, uint32_t attr)
+{
+ if (player != NULL)
+ return player->cb->get_metadata(attr, player->user_data);
+
+ if (attr == AVRCP_MEDIA_ATTRIBUTE_TITLE)
+ return "";
+
+ return NULL;
+}
+
static uint16_t player_write_media_attribute(struct avrcp_player *player,
uint32_t id, uint8_t *buf,
uint16_t *pos,
@@ -489,7 +500,7 @@ static uint16_t player_write_media_attribute(struct avrcp_player *player,

DBG("%u", id);

- value = player->cb->get_metadata(id, player->user_data);
+ value = player_get_metadata(player, id);
if (value == NULL) {
*offset = 0;
return 0;
@@ -588,6 +599,9 @@ static int player_set_attribute(struct avrcp_player *player,
{
DBG("Change attribute: %u %u", attr, val);

+ if (player == NULL)
+ return -ENOENT;
+
return player->cb->set_setting(attr, val, player->user_data);
}

@@ -597,6 +611,9 @@ static int player_get_attribute(struct avrcp_player *player, uint8_t attr)

DBG("attr %u", attr);

+ if (player == NULL)
+ return -ENOENT;
+
value = player->cb->get_setting(attr, player->user_data);
if (value < 0)
DBG("attr %u not supported by player", attr);
@@ -653,7 +670,7 @@ static uint8_t avrcp_handle_list_player_attributes(struct avrcp *session,
uint16_t len = ntohs(pdu->params_len);
unsigned int i;

- if (len != 0 || player == NULL) {
+ if (len != 0) {
pdu->params_len = htons(1);
pdu->params[0] = AVRCP_STATUS_INVALID_PARAM;
return AVC_CTYPE_REJECTED;
@@ -685,7 +702,7 @@ static uint8_t avrcp_handle_list_player_values(struct avrcp *session,
uint16_t len = ntohs(pdu->params_len);
unsigned int i;

- if (len != 1 || player == NULL)
+ if (len != 1)
goto err;

if (player_get_attribute(player, pdu->params[0]) < 0)
@@ -707,6 +724,15 @@ err:
return AVC_CTYPE_REJECTED;
}

+static GList *player_list_metadata(struct avrcp_player *player)
+{
+ if (player != NULL)
+ return player->cb->list_metadata(player->user_data);
+
+ return g_list_prepend(NULL,
+ GUINT_TO_POINTER(AVRCP_MEDIA_ATTRIBUTE_TITLE));
+}
+
static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
struct avrcp_header *pdu,
uint8_t transaction)
@@ -719,7 +745,7 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
GList *attr_ids;
uint16_t offset;

- if (len < 9 || identifier != 0 || player == NULL)
+ if (len < 9 || identifier != 0)
goto err;

nattr = pdu->params[8];
@@ -732,7 +758,7 @@ static uint8_t avrcp_handle_get_element_attributes(struct avrcp *session,
* Return all available information, at least
* title must be returned if there's a track selected.
*/
- attr_ids = player->cb->list_metadata(player->user_data);
+ attr_ids = player_list_metadata(player);
len = g_list_length(attr_ids);
} else {
unsigned int i;
@@ -788,8 +814,7 @@ static uint8_t avrcp_handle_get_current_player_value(struct avrcp *session,
uint8_t *settings;
unsigned int i;

- if (player == NULL || len <= 1 || pdu->params[0] != len - 1 ||
- player == NULL)
+ if (len <= 1 || pdu->params[0] != len - 1)
goto err;

/*
@@ -922,6 +947,14 @@ err:
return AVC_CTYPE_REJECTED;
}

+static uint32_t player_get_position(struct avrcp_player *player)
+{
+ if (player == NULL)
+ return 0;
+
+ return player->cb->get_position(player->user_data);
+}
+
static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
struct avrcp_header *pdu,
uint8_t transaction)
@@ -932,15 +965,15 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
uint32_t duration;
void *pduration;

- if (len != 0 || player == NULL) {
+ if (len != 0) {
pdu->params_len = htons(1);
pdu->params[0] = AVRCP_STATUS_INVALID_PARAM;
return AVC_CTYPE_REJECTED;
}

- position = player->cb->get_position(player->user_data);
- pduration = player->cb->get_metadata(AVRCP_MEDIA_ATTRIBUTE_DURATION,
- player->user_data);
+ position = player_get_position(player);
+ pduration = player_get_metadata(player,
+ AVRCP_MEDIA_ATTRIBUTE_DURATION);
if (pduration != NULL)
duration = htonl(GPOINTER_TO_UINT(pduration));
else
@@ -957,6 +990,22 @@ static uint8_t avrcp_handle_get_play_status(struct avrcp *session,
return AVC_CTYPE_STABLE;
}

+static uint64_t player_get_uid(struct avrcp_player *player)
+{
+ if (player == NULL)
+ return UINT64_MAX;
+
+ return player->cb->get_status(player->user_data);
+}
+
+static uint8_t player_get_status(struct avrcp_player *player)
+{
+ if (player == NULL)
+ return AVRCP_PLAY_STATUS_STOPPED;
+
+ return player->cb->get_status(player->user_data);
+}
+
static uint8_t avrcp_handle_register_notification(struct avrcp *session,
struct avrcp_header *pdu,
uint8_t transaction)
@@ -970,18 +1019,18 @@ static uint8_t avrcp_handle_register_notification(struct avrcp *session,
* one is applicable only for EVENT_PLAYBACK_POS_CHANGED. See AVRCP
* 1.3 spec, section 5.4.2.
*/
- if (len != 5 || player == NULL)
+ if (len != 5)
goto err;

switch (pdu->params[0]) {
case AVRCP_EVENT_STATUS_CHANGED:
len = 2;
- pdu->params[1] = player->cb->get_status(player->user_data);
+ pdu->params[1] = player_get_status(player);

break;
case AVRCP_EVENT_TRACK_CHANGED:
len = 9;
- uid = player->cb->get_uid(player->user_data);
+ uid = player_get_uid(player);
memcpy(&pdu->params[1], &uid, sizeof(uint64_t));

break;
--
1.7.11.4


2012-10-01 11:37:48

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 4/7] AVCTP: Simplify channel handling

From: Luiz Augusto von Dentz <[email protected]>

Make both control and browsing channels to use the same structure to
store its channel information.
---
audio/avctp.c | 159 +++++++++++++++++++++++++++++-----------------------------
1 file changed, 79 insertions(+), 80 deletions(-)

diff --git a/audio/avctp.c b/audio/avctp.c
index 6fd5491..dfc5601 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -131,6 +131,12 @@ struct avctp_rsp_handler {
void *user_data;
};

+struct avctp_channel {
+ GIOChannel *io;
+ guint watch;
+ uint16_t mtu;
+};
+
struct avctp {
struct avctp_server *server;
bdaddr_t dst;
@@ -139,13 +145,8 @@ struct avctp {

int uinput;

- GIOChannel *control_io;
- GIOChannel *browsing_io;
- guint control_io_id;
- guint browsing_io_id;
-
- uint16_t control_mtu;
- uint16_t browsing_mtu;
+ struct avctp_channel *control;
+ struct avctp_channel *browsing;

uint8_t key_quirks[256];
GSList *handlers;
@@ -332,6 +333,17 @@ static struct avctp_pdu_handler *find_handler(GSList *list, uint8_t opcode)
return NULL;
}

+static void avctp_channel_destroy(struct avctp_channel *chan)
+{
+ g_io_channel_shutdown(chan->io, TRUE, NULL);
+ g_io_channel_unref(chan->io);
+
+ if (chan->watch)
+ g_source_remove(chan->watch);
+
+ g_free(chan);
+}
+
static void avctp_disconnected(struct avctp *session)
{
struct avctp_server *server;
@@ -339,36 +351,11 @@ static void avctp_disconnected(struct avctp *session)
if (!session)
return;

- if (session->browsing_io) {
- g_io_channel_shutdown(session->browsing_io, TRUE, NULL);
- g_io_channel_unref(session->browsing_io);
- session->browsing_io = NULL;
- }
-
- if (session->browsing_io_id) {
- g_source_remove(session->browsing_io_id);
- session->browsing_io_id = 0;
- }
-
- if (session->control_io) {
- g_io_channel_shutdown(session->control_io, TRUE, NULL);
- g_io_channel_unref(session->control_io);
- session->control_io = NULL;
- }
-
- if (session->control_io_id) {
- g_source_remove(session->control_io_id);
- session->control_io_id = 0;
+ if (session->browsing)
+ avctp_channel_destroy(session->browsing);

- if (session->state == AVCTP_STATE_CONNECTING) {
- struct audio_device *dev;
-
- dev = manager_get_device(&session->server->src,
- &session->dst, FALSE);
- audio_device_cancel_authorization(dev, auth_cb,
- session);
- }
- }
+ if (session->control)
+ avctp_channel_destroy(session->control);

if (session->uinput >= 0) {
char address[18];
@@ -431,7 +418,7 @@ static void avctp_set_state(struct avctp *session, avctp_state_t new_state)
}
}

-static void handle_response(struct avctp *session, struct avctp_header *avctp,
+static void control_response(struct avctp *session, struct avctp_header *avctp,
struct avc_header *avc, uint8_t *operands,
size_t operand_count)
{
@@ -467,7 +454,7 @@ static gboolean session_browsing_cb(GIOChannel *chan, GIOCondition cond,
if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
goto failed;

- sock = g_io_channel_unix_get_fd(session->browsing_io);
+ sock = g_io_channel_unix_get_fd(chan);

ret = read(sock, buf, sizeof(buf));
if (ret <= 0)
@@ -519,7 +506,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
if (cond & (G_IO_ERR | G_IO_HUP | G_IO_NVAL))
goto failed;

- sock = g_io_channel_unix_get_fd(session->control_io);
+ sock = g_io_channel_unix_get_fd(chan);

ret = read(sock, buf, sizeof(buf));
if (ret <= 0)
@@ -559,7 +546,7 @@ static gboolean session_cb(GIOChannel *chan, GIOCondition cond,
avc->opcode, operand_count);

if (avctp->cr == AVCTP_RESPONSE) {
- handle_response(session, avctp, avc, operands, operand_count);
+ control_response(session, avctp, avc, operands, operand_count);
return TRUE;
}

@@ -687,6 +674,16 @@ static void init_uinput(struct avctp *session)
DBG("AVRCP: uinput initialized for %s", address);
}

+static struct avctp_channel *avctp_channel_create(GIOChannel *io)
+{
+ struct avctp_channel *chan;
+
+ chan = g_new0(struct avctp_channel, 1);
+ chan->io = g_io_channel_ref(io);
+
+ return chan;
+}
+
static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
gpointer data)
{
@@ -697,10 +694,7 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,

if (err) {
error("Browsing: %s", err->message);
- g_io_channel_shutdown(chan, TRUE, NULL);
- g_io_channel_unref(chan);
- session->browsing_io = NULL;
- return;
+ goto fail;
}

bt_io_get(chan, &gerr,
@@ -711,20 +705,28 @@ static void avctp_connect_browsing_cb(GIOChannel *chan, GError *err,
error("%s", gerr->message);
g_io_channel_shutdown(chan, TRUE, NULL);
g_io_channel_unref(chan);
- session->browsing_io = NULL;
g_error_free(gerr);
- return;
+ goto fail;
}

DBG("AVCTP Browsing: connected to %s", address);

- if (!session->browsing_io)
- session->browsing_io = g_io_channel_ref(chan);
+ if (session->browsing == NULL)
+ session->browsing = avctp_channel_create(chan);

- session->browsing_mtu = imtu;
- session->browsing_io_id = g_io_add_watch(chan,
+ session->browsing->mtu = imtu;
+ session->browsing->watch = g_io_add_watch(session->browsing->io,
G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
- (GIOFunc) session_browsing_cb, session); }
+ (GIOFunc) session_browsing_cb, session);
+
+ return;
+
+fail:
+ if (session->browsing) {
+ avctp_channel_destroy(session->browsing);
+ session->browsing = NULL;
+ }
+}

static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)
{
@@ -752,16 +754,17 @@ static void avctp_connect_cb(GIOChannel *chan, GError *err, gpointer data)

DBG("AVCTP: connected to %s", address);

- if (!session->control_io)
- session->control_io = g_io_channel_ref(chan);
+ if (session->control == NULL)
+ session->control = avctp_channel_create(chan);
+
+ session->control->mtu = imtu;
+ session->control->watch = g_io_add_watch(session->control->io,
+ G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
+ (GIOFunc) session_cb, session);

init_uinput(session);

avctp_set_state(session, AVCTP_STATE_CONNECTED);
- session->control_mtu = imtu;
- session->control_io_id = g_io_add_watch(chan,
- G_IO_IN | G_IO_ERR | G_IO_HUP | G_IO_NVAL,
- (GIOFunc) session_cb, session);
}

static void auth_cb(DBusError *derr, void *user_data)
@@ -769,9 +772,9 @@ static void auth_cb(DBusError *derr, void *user_data)
struct avctp *session = user_data;
GError *err = NULL;

- if (session->control_io_id) {
- g_source_remove(session->control_io_id);
- session->control_io_id = 0;
+ if (session->control->watch > 0) {
+ g_source_remove(session->control->watch);
+ session->control->watch = 0;
}

if (derr && dbus_error_is_set(derr)) {
@@ -780,7 +783,7 @@ static void auth_cb(DBusError *derr, void *user_data)
return;
}

- if (!bt_io_accept(session->control_io, avctp_connect_cb, session,
+ if (!bt_io_accept(session->control->io, avctp_connect_cb, session,
NULL, &err)) {
error("bt_io_accept: %s", err->message);
g_error_free(err);
@@ -845,20 +848,20 @@ static struct avctp *avctp_get_internal(const bdaddr_t *src,
static void avctp_control_confirm(struct avctp *session, GIOChannel *chan,
struct audio_device *dev)
{
- if (session->control_io) {
+ if (session->control != NULL) {
error("Control: Refusing unexpected connect");
g_io_channel_shutdown(chan, TRUE, NULL);
return;
}

avctp_set_state(session, AVCTP_STATE_CONNECTING);
- session->control_io = g_io_channel_ref(chan);
+ session->control = avctp_channel_create(chan);

if (audio_device_request_authorization(dev, AVRCP_TARGET_UUID,
auth_cb, session) < 0)
goto drop;

- session->control_io_id = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP |
+ session->control->watch = g_io_add_watch(chan, G_IO_ERR | G_IO_HUP |
G_IO_NVAL, session_cb, session);
return;

@@ -871,7 +874,7 @@ static void avctp_browsing_confirm(struct avctp *session, GIOChannel *chan,
{
GError *err = NULL;

- if (session->control_io == NULL || session->browsing_io) {
+ if (session->control == NULL || session->browsing != NULL) {
error("Browsing: Refusing unexpected connect");
g_io_channel_shutdown(chan, TRUE, NULL);
return;
@@ -918,8 +921,8 @@ static void avctp_confirm_cb(GIOChannel *chan, gpointer data)
DBG("AVCTP: incoming connect from %s", address);

session = avctp_get_internal(&src, &dst);
- if (!session)
- goto drop;
+ if (session == NULL)
+ return;

dev = manager_get_device(&src, &dst, FALSE);
if (!dev) {
@@ -949,13 +952,7 @@ static void avctp_confirm_cb(GIOChannel *chan, gpointer data)
return;

drop:
- if (session && session->browsing_io)
- g_io_channel_unref(session->browsing_io);
-
- if (session && session->control_io)
- g_io_channel_unref(session->control_io);
-
- if (session && psm == AVCTP_CONTROL_PSM)
+ if (psm == AVCTP_CONTROL_PSM)
avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
}

@@ -1093,7 +1090,7 @@ int avctp_send_passthrough(struct avctp *session, uint8_t op)
operands[0] = op & 0x7f;
operands[1] = 0;

- sk = g_io_channel_unix_get_fd(session->control_io);
+ sk = g_io_channel_unix_get_fd(session->control->io);

if (write(sk, buf, sizeof(buf)) < 0)
return -errno;
@@ -1122,7 +1119,7 @@ static int avctp_send(struct avctp *session, uint8_t transaction, uint8_t cr,
if (session->state != AVCTP_STATE_CONNECTED)
return -ENOTCONN;

- sk = g_io_channel_unix_get_fd(session->control_io);
+ sk = g_io_channel_unix_get_fd(session->control->io);

memset(buf, 0, sizeof(buf));

@@ -1306,7 +1303,8 @@ struct avctp *avctp_connect(const bdaddr_t *src, const bdaddr_t *dst)
return NULL;
}

- session->control_io = io;
+ session->control = avctp_channel_create(io);
+ g_io_channel_unref(io);

return session;
}
@@ -1319,7 +1317,7 @@ int avctp_connect_browsing(struct avctp *session)
if (session->state != AVCTP_STATE_CONNECTED)
return -ENOTCONN;

- if (session->browsing_io != NULL)
+ if (session->browsing != NULL)
return 0;

io = bt_io_connect(avctp_connect_browsing_cb, session, NULL, &err,
@@ -1334,14 +1332,15 @@ int avctp_connect_browsing(struct avctp *session)
return -EIO;
}

- session->browsing_io = io;
+ session->browsing = avctp_channel_create(io);
+ g_io_channel_unref(io);

return 0;
}

void avctp_disconnect(struct avctp *session)
{
- if (!session->control_io)
+ if (session->state == AVCTP_STATE_DISCONNECTED)
return;

avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
--
1.7.11.4


2012-10-01 11:37:50

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 6/7] AVRCP: Register to AVCTP state changes without depending on player

From: Luiz Augusto von Dentz <[email protected]>

It is not longer necessary to have a player to be able to register the
extra pdu handlers.
---
audio/avrcp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 2e36fb7..21d105a 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1467,6 +1467,9 @@ int avrcp_register(const bdaddr_t *src, GKeyFile *config)

servers = g_slist_append(servers, server);

+ if (!avctp_id)
+ avctp_id = avctp_add_state_cb(state_changed, NULL);
+
return 0;
}

@@ -1527,9 +1530,6 @@ struct avrcp_player *avrcp_register_player(const bdaddr_t *src,
player->user_data = user_data;
player->destroy = destroy;

- if (!avctp_id)
- avctp_id = avctp_add_state_cb(state_changed, NULL);
-
server->players = g_slist_append(server->players, player);

/* Assign player to session without current player */
--
1.7.11.4


2012-10-01 11:37:47

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 3/7] AVRCP: Fix not removing session from player upon disconnect

From: Luiz Augusto von Dentz <[email protected]>

Invalid read of size 8
at 0x1310E3: avrcp_unregister_player (avrcp.c:1604)
by 0x13EB57: path_free (media.c:1834)
by 0x123208: remove_interface.isra.1 (object.c:558)
by 0x1238DD: g_dbus_unregister_interface (object.c:705)
by 0x124BB8: media_server_remove (manager.c:1077)
by 0x4E91C5C: g_slist_foreach (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x17B349: adapter_remove (adapter.c:2309)
by 0x176F39: manager_cleanup (manager.c:290)
by 0x120E65: main (main.c:555)
Address 0x6685058 is 24 bytes inside a block of size 80 free'd
at 0x4C279AE: free (vg_replace_malloc.c:427)
by 0x4E7C50E: g_free (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x12FC97: state_changed (avrcp.c:1380)
by 0x12D351: avctp_set_state (avctp.c:396)
by 0x12D7B4: session_cb (avctp.c:601)
by 0x4E76824: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4E76B57: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4E76F51: g_main_loop_run (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x120E51: main (main.c:551)
---
audio/avrcp.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 49f1550..2e36fb7 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1328,6 +1328,11 @@ static void state_changed(struct audio_device *dev, avctp_state_t old_state,
avctp_unregister_browsing_pdu_handler(
session->browsing_handler);

+ if (session->player != NULL)
+ session->player->sessions = g_slist_remove(
+ session->player->sessions,
+ session);
+
g_free(session);
break;
case AVCTP_STATE_CONNECTING:
--
1.7.11.4


2012-10-01 11:37:46

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ 2/7] AVRCP: Fix not freeing player session list on exit

From: Luiz Augusto von Dentz <[email protected]>

16 bytes in 1 blocks are definitely lost in loss record 111 of 359
at 0x4A0884D: malloc (vg_replace_malloc.c:263)
by 0x4C8026E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4C94671: g_slice_alloc (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4C959B2: g_slist_append (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x130FCC: avrcp_register_player (avrcp.c:1584)
by 0x13FA1F: register_player (media.c:1689)
by 0x123100: process_message.isra.0 (object.c:197)
by 0x4F70684: ??? (in /usr/lib64/libdbus-1.so.3.5.6)
by 0x4F6290C: dbus_connection_dispatch (in /usr/lib64/libdbus-1.so.3.5.6)
by 0x121747: message_dispatch (mainloop.c:76)
by 0x4C7B22A: ??? (in /usr/lib64/libglib-2.0.so.0.3200.4)
by 0x4C7A694: g_main_context_dispatch (in /usr/lib64/libglib-2.0.so.0.3200.4)
---
audio/avrcp.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 376f4a1..49f1550 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1472,6 +1472,7 @@ static void player_destroy(gpointer data)
if (player->destroy)
player->destroy(player->user_data);

+ g_slist_free(player->sessions);
g_free(player);
}

--
1.7.11.4