Re: [PATCH] winealsa: snd_card_get_name uses strdup (Valgrind).

[Christian Costa]

In that case you don't need the if (!err) condition to free the buffer
otherwise you leak the string return by strdup.

Christian

2012/2/1 Lauri Kenttä <lauri.kentta(a)gmail.com>
> ---
>  dlls/winealsa.drv/mmdevdrv.c |   10 +++++++---
>  1 files changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/dlls/winealsa.drv/mmdevdrv.c b/dlls/winealsa.drv/mmdevdrv.c
> index 0bedcc5..e434038 100644
> --- a/dlls/winealsa.drv/mmdevdrv.c
> +++ b/dlls/winealsa.drv/mmdevdrv.c
> @@ -363,7 +363,7 @@ static HRESULT alsa_enum_devices(EDataFlow flow, WCHAR
> **ids, char **keys,
>     for(err = snd_card_next(&card); card != -1 && err >= 0;
>             err = snd_card_next(&card)){
>         char cardpath[64];
> -        const char *cardname;
> +        char *cardname;
>         WCHAR *cardnameW;
>         snd_ctl_t *ctl;
>         DWORD len;
> @@ -376,16 +376,18 @@ static HRESULT alsa_enum_devices(EDataFlow flow,
> WCHAR **ids, char **keys,
>             continue;
>         }
>
> -        if((err = snd_card_get_name(card, (char **)&cardname)) < 0){
> +        if((err = snd_card_get_name(card, &cardname)) < 0){
>             WARN("Unable to get card name for ALSA device %s: %d (%s)\n",
>                     cardpath, err, snd_strerror(err));
>             /* FIXME: Should be localized */
> -            cardname = "Unknown soundcard";
> +            cardname = strdup("Unknown soundcard");
>         }
>
> len = MultiByteToWideChar(CP_UNIXCP, 0, cardname, -1, NULL, 0);
>         cardnameW = HeapAlloc(GetProcessHeap(), 0, len * sizeof(WCHAR));
>         if(!cardnameW){
> +            if(!err)
> +                free(cardname);
>             snd_ctl_close(ctl);
>             return E_OUTOFMEMORY;
>         }
> @@ -394,6 +396,8 @@ static HRESULT alsa_enum_devices(EDataFlow flow, WCHAR
> **ids, char **keys,
>         alsa_get_card_devices(stream, ids, keys, num, ctl, card,
> cardnameW);
>
> HeapFree(GetProcessHeap(), 0, cardnameW);
> +        if(!err)
> +            free(cardname);
>
> snd_ctl_close(ctl);
>     }
> --
> 1.7.9

[PATCH] winealsa: snd_card_get_name uses strdup (Valgrind).

[Lauri Kenttä]

On 2012-02-01 17:00, Christian Costa wrote:
> In that case you don't need the if (!err) condition to free the 
> buffer
> otherwise you leak the string return by strdup.

Yeah, I didn't think this through and screwed up. :P

I also noticed now that Jörg Höhle already deemed this strdup solution 
ugly in his previous try. Luckily Michael Stefaniuc sent a finer patch 
#83376.

-- 
Lauri Kenttä

[PATCH] winealsa: snd_card_get_name uses strdup (Valgrind).

[Michael Stefaniuc]

Lauri Kenttä wrote:
> On 2012-02-01 17:00, Christian Costa wrote:
> > In that case you don't need the if (!err) condition to free the buffer
> > otherwise you leak the string return by strdup.
>
> Yeah, I didn't think this through and screwed up. :P
>
> I also noticed now that Jörg Höhle already deemed this strdup solution
> ugly in his previous try. Luckily Michael Stefaniuc sent a finer patch
> #83376.
> Still not good enough. But at least we forced Alexandre to fix it himself ;)

bye
	michael