[PATCH] ASoC: core: Use devm_kzalloc() instead kzalloc()

[PATCH] ASoC: core: Use devm_kzalloc() instead kzalloc()

[Xiubo Li]

Makes the code slightly shorter

Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>
---
 sound/soc/soc-generic-dmaengine-pcm.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
index 5bace12..bfb012f 100644
--- a/sound/soc/soc-generic-dmaengine-pcm.c
+++ b/sound/soc/soc-generic-dmaengine-pcm.c
@@ -388,7 +388,7 @@ int snd_dmaengine_pcm_register(struct device *dev,
 	struct dmaengine_pcm *pcm;
 	int ret;
 
-	pcm = kzalloc(sizeof(*pcm), GFP_KERNEL);
+	pcm = devm_kzalloc(dev, sizeof(*pcm), GFP_KERNEL);
 	if (!pcm)
 		return -ENOMEM;
 
@@ -408,7 +408,6 @@ int snd_dmaengine_pcm_register(struct device *dev,
 
 err_free_dma:
 	dmaengine_pcm_release_chan(pcm);
-	kfree(pcm);
 	return ret;
 }
 EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_register);
@@ -433,7 +432,6 @@ void snd_dmaengine_pcm_unregister(struct device *dev)
 
 	snd_soc_remove_platform(platform);
 	dmaengine_pcm_release_chan(pcm);
-	kfree(pcm);
 }
 EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_unregister);
 
-- 
1.8.4

Re: [alsa-devel] [PATCH] ASoC: core: Use devm_kzalloc() instead kzalloc()

[Lars-Peter Clausen]

On 01/16/2014 09:44 AM, Xiubo Li wrote:
> Makes the code slightly shorter
>
> Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>

I don't like this. I don't think it is a good design pattern to call devm 
function from within (especial non-devm) library functions. It creates an 
asymmetric API. The memory is allocated when snd_dmaengine_pcm_register() is 
called, but it is not freed when  snd_dmaengine_pcm_unregister() is called. 
This goes against the principle of least surprise.

- Lars

> ---
>   sound/soc/soc-generic-dmaengine-pcm.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
> index 5bace12..bfb012f 100644
> --- a/sound/soc/soc-generic-dmaengine-pcm.c
> +++ b/sound/soc/soc-generic-dmaengine-pcm.c
> @@ -388,7 +388,7 @@ int snd_dmaengine_pcm_register(struct device *dev,
>   	struct dmaengine_pcm *pcm;
>   	int ret;
>
> -	pcm = kzalloc(sizeof(*pcm), GFP_KERNEL);
> +	pcm = devm_kzalloc(dev, sizeof(*pcm), GFP_KERNEL);
>   	if (!pcm)
>   		return -ENOMEM;
>
> @@ -408,7 +408,6 @@ int snd_dmaengine_pcm_register(struct device *dev,
>
>   err_free_dma:
>   	dmaengine_pcm_release_chan(pcm);
> -	kfree(pcm);
>   	return ret;
>   }
>   EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_register);
> @@ -433,7 +432,6 @@ void snd_dmaengine_pcm_unregister(struct device *dev)
>
>   	snd_soc_remove_platform(platform);
>   	dmaengine_pcm_release_chan(pcm);
> -	kfree(pcm);
>   }
>   EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_unregister);
>
>

Re: [alsa-devel] [PATCH] ASoC: core: Use devm_kzalloc() instead kzalloc()

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 678 bytes --]

On Thu, Jan 16, 2014 at 10:52:35PM +0100, Lars-Peter Clausen wrote:
> On 01/16/2014 09:44 AM, Xiubo Li wrote:
> >Makes the code slightly shorter

> >Signed-off-by: Xiubo Li <Li.Xiubo@freescale.com>

> I don't like this. I don't think it is a good design pattern to call
> devm function from within (especial non-devm) library functions. It
> creates an asymmetric API. The memory is allocated when
> snd_dmaengine_pcm_register() is called, but it is not freed when
> snd_dmaengine_pcm_unregister() is called. This goes against the
> principle of least surprise.

Yes, I tend to agree - unless we only support managed registration the
API shouldn't do managed things internally.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

RE: [alsa-devel] [PATCH] ASoC: core: Use devm_kzalloc() instead kzalloc()

[Li.Xiubo]

Hi Mar, Lars


> > I don't like this. I don't think it is a good design pattern to call
> > devm function from within (especial non-devm) library functions. It
> > creates an asymmetric API. The memory is allocated when
> > snd_dmaengine_pcm_register() is called, but it is not freed when
> > snd_dmaengine_pcm_unregister() is called. This goes against the
> > principle of least surprise.
> 
> Yes, I tend to agree - unless we only support managed registration the
> API shouldn't do managed things internally.

Got it.

Thanks,