Re: [PATCH] ALSA: ASoC: Fix memory leaks for the CS4270 driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Takashi Iwai wrote:

> - In cs4270_remove(), cs4270_i2c_detach() is called after
>   snd_soc_free_pcms().  snd_soc_free_pcms() invokes snd_card_free()
>   inside, and this releases the all resources already, including the
>   control elements.  That is, you free an already-freed item.

I was wondering why so few drivers were calling snd_ctl_remove().  :-)

>   Well, looking the code again, you create only one element, and if
>   this fails, there is no other remaining element.  Thus, there is
>   nothing to free there.

True, but I intend on adding more controls in the future, so I want to code to
handle that automatically.

> 
> - The way you look for a kcontrol is wrong although it may work in
>   practice.  A usual way is to remember the kctl and use it for
>   removal, or find the kctl via snd_ctl_find_id(), not _numid().

It would be nice if snd_kcontrol_new had a snd_kcontrol *.  I could use it to
store the pointer for later deallocation.  Would you accept a patch to add that?

> - Your patch merged in the ALSA tree already would conflict with this
>   patch.  This is a mess, results in the whole rebase of git tree.

What, wait other patch?  I based this patch on
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-2.6.git.  It should
apply cleanly on that repo.

> So, I think the code in 2.6.27 tree can stay as is.  There is no leak
> in the end.  But, we can fix cs4270.c in the latest ALSA tree,
> e.g. add a missing remove callback to release codec->reg_cache.

I'm confused, because I thought I was fixing the latest ALSA tree.

-- 
Timur Tabi
Linux kernel developer at Freescale
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux