linux-kernel - Re: [alsa-devel] [PATCH] ASoC: core: Use devm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <52D854A3.5090109@metafoo.de>
Date:	Thu, 16 Jan 2014 22:52:35 +0100
From:	Lars-Peter Clausen <lars@...afoo.de>
To:	Xiubo Li <Li.Xiubo@...escale.com>
CC:	lgirdwood@...il.com, broonie@...nel.org, perex@...ex.cz,
	tiwai@...e.de, alsa-devel@...a-project.org,
	linux-kernel@...r.kernel.org
Subject: Re: [alsa-devel] [PATCH] ASoC: core: Use devm_kzalloc() instead kzalloc()

On 01/16/2014 09:44 AM, Xiubo Li wrote:
> Makes the code slightly shorter
>
> Signed-off-by: Xiubo Li <Li.Xiubo@...escale.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);
>
>

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/