2011-10-03 14:57:50

by Luiz Augusto von Dentz

Subject: [PATCH BlueZ 1/2] Fix regression when disconnecting AVRCP

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

This prevent to disconnect AVRCP when doing e.g. Audio.Disconnect
---
audio/avctp.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/audio/avctp.c b/audio/avctp.c
index e9b8e40..b8cb36e 100644
--- a/audio/avctp.c
+++ b/audio/avctp.c
@@ -1021,7 +1021,7 @@ struct avctp *avctp_connect(const bdaddr_t *src, const bdaddr_t *dst)

void avctp_disconnect(struct avctp *session)
{
- if (session->io)
+ if (!session->io)
return;

avctp_set_state(session, AVCTP_STATE_DISCONNECTED);
--
1.7.6.2

2011-10-03 19:24:04

by Lucas De Marchi

Subject: Re: [PATCH BlueZ 1/2] Fix regression when disconnecting AVRCP

Hi,

On Mon, Oct 3, 2011 at 11:57 AM, Luiz Augusto von Dentz
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> This prevent to disconnect AVRCP when doing e.g. Audio.Disconnect
> ---
> =A0audio/avctp.c | =A0 =A02 +-
> =A01 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/audio/avctp.c b/audio/avctp.c
> index e9b8e40..b8cb36e 100644
> --- a/audio/avctp.c
> +++ b/audio/avctp.c
> @@ -1021,7 +1021,7 @@ struct avctp *avctp_connect(const bdaddr_t *src, co=
nst bdaddr_t *dst)
>
> =A0void avctp_disconnect(struct avctp *session)
> =A0{
> - =A0 =A0 =A0 if (session->io)
> + =A0 =A0 =A0 if (!session->io)

At least in oFono we have a coding style rule to always compare
pointers to NULL. I even have an old patch in bluez/gdbus for this:
6e66423fe87441e3e61e5a325fde1515efa63ac5

Luiz, Johan, Marcel: wouldn't it be better if BlueZ used this rule,
too? (and put the rules somewhere like oFono does?)



Lucas De Marchi

2011-10-03 16:43:54

by Johan Hedberg

Subject: Re: [PATCH BlueZ 2/2] Fix possible crash when player is destroyed

Hi Luiz,

On Mon, Oct 03, 2011, Luiz Augusto von Dentz wrote:
> On Mon, Oct 3, 2011 at 5:57 PM, Luiz Augusto von Dentz
> <[email protected]> wrote:
> > From: Luiz Augusto von Dentz <[email protected]>
> >
> > When player is unregistered/destroyed its pdu handler should also be
> > removed.
> > ---
> > ?audio/avrcp.c | ? ?3 +++
> > ?1 files changed, 3 insertions(+), 0 deletions(-)
> >
> > diff --git a/audio/avrcp.c b/audio/avrcp.c
> > index 5aa8dc5..9a73ff4 100644
> > --- a/audio/avrcp.c
> > +++ b/audio/avrcp.c
> > @@ -1101,6 +1101,9 @@ static void player_destroy(gpointer data)
> > ? ? ? ?if (player->destroy)
> > ? ? ? ? ? ? ? ?player->destroy(player->user_data);
> >
> > + ? ? ? if (player->handler)
> > + ? ? ? ? ? ? ? avctp_unregister_pdu_handler(player->handler);
> > +
> > ? ? ? ?g_free(player);
> > ?}
> >
> > --
> > 1.7.6.2
>
> Please skip this since player_destroy is not even upstream yet.

Ok. The first patch has anyway been pushed upstream.

Johan

2011-10-03 16:37:12

by Luiz Augusto von Dentz

Subject: Re: [PATCH BlueZ 2/2] Fix possible crash when player is destroyed

Hi,

On Mon, Oct 3, 2011 at 5:57 PM, Luiz Augusto von Dentz
<[email protected]> wrote:
> From: Luiz Augusto von Dentz <[email protected]>
>
> When player is unregistered/destroyed its pdu handler should also be
> removed.
> ---
> ?audio/avrcp.c | ? ?3 +++
> ?1 files changed, 3 insertions(+), 0 deletions(-)
>
> diff --git a/audio/avrcp.c b/audio/avrcp.c
> index 5aa8dc5..9a73ff4 100644
> --- a/audio/avrcp.c
> +++ b/audio/avrcp.c
> @@ -1101,6 +1101,9 @@ static void player_destroy(gpointer data)
> ? ? ? ?if (player->destroy)
> ? ? ? ? ? ? ? ?player->destroy(player->user_data);
>
> + ? ? ? if (player->handler)
> + ? ? ? ? ? ? ? avctp_unregister_pdu_handler(player->handler);
> +
> ? ? ? ?g_free(player);
> ?}
>
> --
> 1.7.6.2

Please skip this since player_destroy is not even upstream yet.

--
Luiz Augusto von Dentz

2011-10-03 14:57:51

by Luiz Augusto von Dentz

Subject: [PATCH BlueZ 2/2] Fix possible crash when player is destroyed

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

When player is unregistered/destroyed its pdu handler should also be
removed.
---
audio/avrcp.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/audio/avrcp.c b/audio/avrcp.c
index 5aa8dc5..9a73ff4 100644
--- a/audio/avrcp.c
+++ b/audio/avrcp.c
@@ -1101,6 +1101,9 @@ static void player_destroy(gpointer data)
if (player->destroy)
player->destroy(player->user_data);

+ if (player->handler)
+ avctp_unregister_pdu_handler(player->handler);
+
g_free(player);
}

--
1.7.6.2