diff mbox

[1/3] ASoC: simple-card: Use devm_kcalloc() in asoc_simple_card_parse_aux_devs()

Message ID 8c8c6979-0368-9ef5-2e10-57199f0bc03e@users.sourceforge.net (mailing list archive)
State New, archived
Headers show

Commit Message

SF Markus Elfring April 26, 2017, 1:04 p.m. UTC
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 26 Apr 2017 14:21:51 +0200

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus use the corresponding function "devm_kcalloc".

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 sound/soc/generic/simple-card.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Alexandre Belloni April 28, 2017, 8:57 a.m. UTC | #1
Hi,

On 26/04/2017 at 15:04:11 +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 26 Apr 2017 14:21:51 +0200
> 
> A multiplication for the size determination of a memory allocation
> indicated that an array data structure should be processed.
> Thus use the corresponding function "devm_kcalloc".
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  sound/soc/generic/simple-card.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
> index 2c9dedab5184..b59d51374b89 100644
> --- a/sound/soc/generic/simple-card.c
> +++ b/sound/soc/generic/simple-card.c
> @@ -334,8 +334,8 @@ static int asoc_simple_card_parse_aux_devs(struct device_node *node,
>  	if (n <= 0)
>  		return -EINVAL;
>  
> -	card->aux_dev = devm_kzalloc(dev,
> -			n * sizeof(*card->aux_dev), GFP_KERNEL);
> +	card->aux_dev = devm_kcalloc(dev, n, sizeof(*card->aux_dev),
> +				     GFP_KERNEL);

Do you realize that this change has absolutely no value and just makes
the code slower (one more test in the allocation path)?
SF Markus Elfring April 28, 2017, 10:48 a.m. UTC | #2
>> @@ -334,8 +334,8 @@ static int asoc_simple_card_parse_aux_devs(struct device_node *node,
>>  	if (n <= 0)
>>  		return -EINVAL;
>>  
>> -	card->aux_dev = devm_kzalloc(dev,
>> -			n * sizeof(*card->aux_dev), GFP_KERNEL);
>> +	card->aux_dev = devm_kcalloc(dev, n, sizeof(*card->aux_dev),
>> +				     GFP_KERNEL);
> 
> Do you realize that this change has absolutely no value

We can have different software development opinions about such a source code adjustment.
Does it improve the indentation for the parameters which are passed to this function call?


> and just makes the code slower (one more test in the allocation path)?

Do we stumble on a target conflict for the shown implementation detail?

Does the previous size calculation contain the general possibility for
an integer overflow?
https://cwe.mitre.org/data/definitions/190.html

The value for the variable “len” (and also “n”) might be small enough so that
the computed value will usually not exceed the data type limit in this use case.

Regards,
Markus
diff mbox

Patch

diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 2c9dedab5184..b59d51374b89 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -334,8 +334,8 @@  static int asoc_simple_card_parse_aux_devs(struct device_node *node,
 	if (n <= 0)
 		return -EINVAL;
 
-	card->aux_dev = devm_kzalloc(dev,
-			n * sizeof(*card->aux_dev), GFP_KERNEL);
+	card->aux_dev = devm_kcalloc(dev, n, sizeof(*card->aux_dev),
+				     GFP_KERNEL);
 	if (!card->aux_dev)
 		return -ENOMEM;