[PATCH] audio: fix memory leak with typefinding

[PATCH] audio: fix memory leak with typefinding

[harri.mahonen]

From: Harri Mahonen <harri.mahonen@gmail.com>

sbc structure gets leaked each time when there is no data or SBC
syncword, because sbc_finalize is not called. Call sbc_init after
checking the data for syncword.

Signed-off-by: Harri Mahonen <harri.mahonen@gmail.com>
---
 audio/gstbluetooth.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/audio/gstbluetooth.c b/audio/gstbluetooth.c
index 26dd4a5..11aefd7 100644
--- a/audio/gstbluetooth.c
+++ b/audio/gstbluetooth.c
@@ -50,10 +50,10 @@ static void sbc_typefind(GstTypeFind *tf, gpointer ignore)
 	sbc_t sbc;
 	guint8 *data = gst_type_find_peek(tf, 0, 32);
 
-	if (sbc_init(&sbc, 0) < 0)
+	if (data == NULL || *data != 0x9c)	/* SBC syncword */
 		return;
 
-	if (data == NULL || *data != 0x9c)	/* SBC syncword */
+	if (sbc_init(&sbc, 0) < 0)
 		return;
 
 	aux = g_new(guint8, 32);
-- 
1.7.0.4

Re: [PATCH] audio: fix memory leak with typefinding

[Luiz Augusto von Dentz]

Hi,

On Wed, Jun 23, 2010 at 10:10 PM,  <harri.mahonen@gmail.com> wrote:
> From: Harri Mahonen <harri.mahonen@gmail.com>
>
> sbc structure gets leaked each time when there is no data or SBC
> syncword, because sbc_finalize is not called. Call sbc_init after
> checking the data for syncword.
>
> Signed-off-by: Harri Mahonen <harri.mahonen@gmail.com>
> ---
>  audio/gstbluetooth.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/audio/gstbluetooth.c b/audio/gstbluetooth.c
> index 26dd4a5..11aefd7 100644
> --- a/audio/gstbluetooth.c
> +++ b/audio/gstbluetooth.c
> @@ -50,10 +50,10 @@ static void sbc_typefind(GstTypeFind *tf, gpointer ignore)
>        sbc_t sbc;
>        guint8 *data = gst_type_find_peek(tf, 0, 32);
>
> -       if (sbc_init(&sbc, 0) < 0)
> +       if (data == NULL || *data != 0x9c)      /* SBC syncword */
>                return;
>
> -       if (data == NULL || *data != 0x9c)      /* SBC syncword */
> +       if (sbc_init(&sbc, 0) < 0)
>                return;
>
>        aux = g_new(guint8, 32);

We might want to have this fix inside sbc_init instead since others
projects like pulseaudio may run into the same problem, also I guess
it is a good practice to free any data allocated when returning an
error.


-- 
Luiz Augusto von Dentz
Computer Engineer

Re: [PATCH] audio: fix memory leak with typefinding

[Luiz Augusto von Dentz]

Hi,

On Thu, Jun 24, 2010 at 10:52 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Hi,
>
> On Wed, Jun 23, 2010 at 10:10 PM,  <harri.mahonen@gmail.com> wrote:
>> From: Harri Mahonen <harri.mahonen@gmail.com>
>>
>> sbc structure gets leaked each time when there is no data or SBC
>> syncword, because sbc_finalize is not called. Call sbc_init after
>> checking the data for syncword.
>>
>> Signed-off-by: Harri Mahonen <harri.mahonen@gmail.com>
>> ---
>>  audio/gstbluetooth.c |    4 ++--
>>  1 files changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/audio/gstbluetooth.c b/audio/gstbluetooth.c
>> index 26dd4a5..11aefd7 100644
>> --- a/audio/gstbluetooth.c
>> +++ b/audio/gstbluetooth.c
>> @@ -50,10 +50,10 @@ static void sbc_typefind(GstTypeFind *tf, gpointer ignore)
>>        sbc_t sbc;
>>        guint8 *data = gst_type_find_peek(tf, 0, 32);
>>
>> -       if (sbc_init(&sbc, 0) < 0)
>> +       if (data == NULL || *data != 0x9c)      /* SBC syncword */
>>                return;
>>
>> -       if (data == NULL || *data != 0x9c)      /* SBC syncword */
>> +       if (sbc_init(&sbc, 0) < 0)
>>                return;
>>
>>        aux = g_new(guint8, 32);
>
> We might want to have this fix inside sbc_init instead since others
> projects like pulseaudio may run into the same problem, also I guess
> it is a good practice to free any data allocated when returning an
> error.

Sorry, I misunderstood the problem, it is really gstreamer only issue,
but I would suggest using the return of sbc_parse so we don't have to
hardcode sbc sync word detection, something like the following:

diff --git a/audio/gstbluetooth.c b/audio/gstbluetooth.c
index 26dd4a5..9930820 100644
--- a/audio/gstbluetooth.c
+++ b/audio/gstbluetooth.c
@@ -50,21 +50,24 @@ static void sbc_typefind(GstTypeFind *tf, gpointer ignore)
        sbc_t sbc;
        guint8 *data = gst_type_find_peek(tf, 0, 32);

-       if (sbc_init(&sbc, 0) < 0)
+       if (data == NULL)
                return;

-       if (data == NULL || *data != 0x9c)      /* SBC syncword */
+       if (sbc_init(&sbc, 0) < 0)
                return;

        aux = g_new(guint8, 32);
        memcpy(aux, data, 32);
-       sbc_parse(&sbc, aux, 32);
-       g_free(aux);
-       caps = gst_sbc_parse_caps_from_sbc(&sbc);
-       sbc_finish(&sbc);
+       if (sbc_parse(&sbc, aux, 32) < 0)
+               goto done;

+       caps = gst_sbc_parse_caps_from_sbc(&sbc);
        gst_type_find_suggest(tf, GST_TYPE_FIND_POSSIBLE, caps);
        gst_caps_unref(caps);
+
+done:
+       g_free(aux);
+       sbc_finish(&sbc);
 }

-- 
Luiz Augusto von Dentz
Computer Engineer

Re: [PATCH] audio: fix memory leak with typefinding

[Harri Mähönen]

On Thu, Jun 24, 2010 at 11:09 AM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Sorry, I misunderstood the problem, it is really gstreamer only issue,
> but I would suggest using the return of sbc_parse so we don't have to
> hardcode sbc sync word detection, something like the following:

Thank you for your comments, I agree with using sbc_parse to avoid
hardcoded sbc sync word detection. I'll repost updated patch.

Br,
Harri