[alsa-devel] [PATCH] ALSA: ASoC: Fix memory leaks for the CS4270 driver

[Takashi Iwai]

At Thu, 04 Sep 2008 10:54:28 -0500,
Timur Tabi wrote:
> 
> 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?

Hmm..  Basically struct snd_kcontrol_new isn't for writing but only
for reading.  Maybe easier is to write a function to get snd_kcontrol
pointer from the given snd_kcontrol_new.

> > - 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.

If the fix needs to go to 2.6.27, you need the patch against 2.6.27-rc5.
And, this patch would conflict with the latest ALSA tree.

> > 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.

Well, the latest ALSA tree != 2.6.27.
The question is whether we need a fix for 2.6.27 or not.


thanks,

Takashi