All of lore.kernel.org
 help / color / mirror / Atom feed
From: SF Markus Elfring <elfring@users.sourceforge.net>
To: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: alsa-devel@alsa-project.org, Jaroslav Kysela <perex@perex.cz>,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Nikita Yushchenko <nikita.yoush@cogentembedded.com>,
	Takashi Iwai <tiwai@suse.com>,
	kernel-janitors@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: [alsa-devel] [PATCH 1/3] ASoC: simple-card: Use devm_kcalloc() in asoc_simple_card_parse_aux_devs()
Date: Fri, 28 Apr 2017 12:48:15 +0200	[thread overview]
Message-ID: <49bbbcac-f970-bbb9-c66b-c9c25229cc6a@users.sourceforge.net> (raw)
In-Reply-To: <20170428085735.i7xpxph24t5v5f5q@piout.net>

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

WARNING: multiple messages have this Message-ID (diff)
From: SF Markus Elfring <elfring@users.sourceforge.net>
To: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Nikita Yushchenko <nikita.yoush@cogentembedded.com>,
	alsa-devel@alsa-project.org,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Takashi Iwai <tiwai@suse.com>,
	kernel-janitors@vger.kernel.org,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [alsa-devel] [PATCH 1/3] ASoC: simple-card: Use devm_kcalloc() in asoc_simple_card_parse_aux_dev
Date: Fri, 28 Apr 2017 10:48:15 +0000	[thread overview]
Message-ID: <49bbbcac-f970-bbb9-c66b-c9c25229cc6a@users.sourceforge.net> (raw)
In-Reply-To: <20170428085735.i7xpxph24t5v5f5q@piout.net>

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

WARNING: multiple messages have this Message-ID (diff)
From: SF Markus Elfring <elfring@users.sourceforge.net>
To: Alexandre Belloni <alexandre.belloni@free-electrons.com>
Cc: Nikita Yushchenko <nikita.yoush@cogentembedded.com>,
	alsa-devel@alsa-project.org,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Takashi Iwai <tiwai@suse.com>,
	kernel-janitors@vger.kernel.org,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>
Subject: Re: [PATCH 1/3] ASoC: simple-card: Use devm_kcalloc() in asoc_simple_card_parse_aux_devs()
Date: Fri, 28 Apr 2017 12:48:15 +0200	[thread overview]
Message-ID: <49bbbcac-f970-bbb9-c66b-c9c25229cc6a@users.sourceforge.net> (raw)
In-Reply-To: <20170428085735.i7xpxph24t5v5f5q@piout.net>

>> @@ -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
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

  reply	other threads:[~2017-04-28 10:49 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-26 13:02 [PATCH 0/3] ASoC: simple-card: Fine-tuning for three function implementations SF Markus Elfring
2017-04-26 13:02 ` SF Markus Elfring
2017-04-26 13:02 ` SF Markus Elfring
2017-04-26 13:04 ` [PATCH 1/3] ASoC: simple-card: Use devm_kcalloc() in asoc_simple_card_parse_aux_devs() SF Markus Elfring
2017-04-26 13:04   ` SF Markus Elfring
2017-04-26 13:04   ` SF Markus Elfring
2017-04-28  8:57   ` [alsa-devel] " Alexandre Belloni
2017-04-28  8:57     ` Alexandre Belloni
2017-04-28  8:57     ` [alsa-devel] [PATCH 1/3] ASoC: simple-card: Use devm_kcalloc() in asoc_simple_card_parse_aux_dev Alexandre Belloni
2017-04-28 10:48     ` SF Markus Elfring [this message]
2017-04-28 10:48       ` [PATCH 1/3] ASoC: simple-card: Use devm_kcalloc() in asoc_simple_card_parse_aux_devs() SF Markus Elfring
2017-04-28 10:48       ` [alsa-devel] [PATCH 1/3] ASoC: simple-card: Use devm_kcalloc() in asoc_simple_card_parse_aux_dev SF Markus Elfring
2017-04-26 13:05 ` [PATCH 2/3] ASoC: simple-card: Use devm_kcalloc() in asoc_simple_card_probe() SF Markus Elfring
2017-04-26 13:05   ` SF Markus Elfring
2017-04-26 13:05   ` SF Markus Elfring
2017-04-26 13:06 ` [PATCH 3/3] ASoC: simple-scu-card: " SF Markus Elfring
2017-04-26 13:06   ` SF Markus Elfring
2017-04-26 13:06   ` SF Markus Elfring

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49bbbcac-f970-bbb9-c66b-c9c25229cc6a@users.sourceforge.net \
    --to=elfring@users.sourceforge.net \
    --cc=alexandre.belloni@free-electrons.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=kuninori.morimoto.gx@renesas.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=nikita.yoush@cogentembedded.com \
    --cc=perex@perex.cz \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.