All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cezary Rojewski <cezary.rojewski@intel.com>
To: Ethan Carter Edwards <ethan@ethancedwards.com>
Cc: <gustavoars@kernel.org>, <linux-sound@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-hardening@vger.kernel.org>,
	"Liam Girdwood" <liam.r.girdwood@linux.intel.com>,
	Peter Ujfalusi <peter.ujfalusi@linux.intel.com>,
	Bard Liao <yung-chuan.liao@linux.intel.com>,
	Ranjani Sridharan <ranjani.sridharan@linux.intel.com>,
	Kai Vehmanen <kai.vehmanen@linux.intel.com>,
	Pierre-Louis Bossart <pierre-louis.bossart@linux.dev>,
	Mark Brown <broonie@kernel.org>,
	"Jaroslav Kysela" <perex@perex.cz>, Takashi Iwai <tiwai@suse.com>
Subject: Re: [PATCH 0/4] ASoC: Intel: avs: move devm_kzalloc(..., size * n, ...) to devm_kcalloc
Date: Mon, 17 Mar 2025 11:28:16 +0100	[thread overview]
Message-ID: <89f83133-eace-4bec-804a-198dee9ae14c@intel.com> (raw)
In-Reply-To: <20250314-sound-avs-kcalloc-v1-0-985f2734c020@ethancedwards.com>

On 2025-03-14 3:38 PM, Ethan Carter Edwards wrote:
> Open coded arithmetic in allocator arguments is discouraged. Helper
> functions like kcalloc or, in this case, devm_kcalloc are preferred. Not
> only for readability purposes but safety purposes.
> 
> The changes move `devm_kzalloc(dev, sizeof(var) * n, GFP_KERNEL)` to
> the helper function `devm_kcalloc(dev, n, sizeof(var), GFP_KERNEL)`.
> 
> Here is a series of four patches within the Intel/avs drivers that make
> these changes. They are all compile tested only but should have no
> effect on runtime behaviour.
>      
> Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
> Link: https://github.com/KSPP/linux/issues/162
> 
> Signed-off-by: Ethan Carter Edwards <ethan@ethancedwards.com>

Hi Ethan,

Thank you for suggestions.  The code looks good, I'd work a bit on the 
presentation though.  For the avs/boards, please keep the title cohesive 
i.e.: do not drop 'avs' scope:

	ASoC: Intel: avs: ssm4567: (the title here)

Do that for all the avs/boards/ patches.

Now, in regard to the title wording - please do reduce the number of 
special characters when possible.  It makes it harder for kernel 
developers to grep for things.  At the same time, being cohesive e.g.: 
'()' when mentioning functions makes it easier to differentiate 
functions from types.  What I'd suggest is:

	ASoC: Intel: avs: ssm4567: Replace devm_kzalloc() with devm_kcalloc()

Plainly states _what_ is being done. Your commit messages plainly answer 
_why_ do the changes so no update needed there : )

--
Nitpick:

Signing off cover letters is redundant.
Also, I'd suggest to leave 'Link:' tag for the lore.kernel.org links and 
operate on references instead, e.g.:

	Open coded arithmetic in allocator arguments is discouraged [1].
	(...)

	[1]: 
https://www.kernel.org/doc/html/latest/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments


Kind regards,
Czarek

> ---
> Ethan Carter Edwards (4):
>        ASoC: Intel: avs: move devm_kzalloc(..., size * n, ...) to devm_kcalloc
>        ASoC: Intel: ssm4567: move devm_kzalloc(..., size * n, ...) to devm_kcalloc
>        ASoC: Intel: max98373: move devm_kzalloc(..., size * n, ...) to devm_kcalloc
>        ASoC: Intel: max98927: move devm_kzalloc(..., size * n, ...) to devm_kcalloc
> 
>   sound/soc/intel/avs/boards/max98373.c | 2 +-
>   sound/soc/intel/avs/boards/max98927.c | 2 +-
>   sound/soc/intel/avs/boards/ssm4567.c  | 2 +-
>   sound/soc/intel/avs/pcm.c             | 2 +-
>   4 files changed, 4 insertions(+), 4 deletions(-)
> ---
> base-commit: da920b7df701770e006928053672147075587fb2
> change-id: 20250314-sound-avs-kcalloc-91cedbc47074
> 
> Best regards,


      parent reply	other threads:[~2025-03-17 10:28 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-14 14:38 [PATCH 0/4] ASoC: Intel: avs: move devm_kzalloc(..., size * n, ...) to devm_kcalloc Ethan Carter Edwards
2025-03-14 14:38 ` [PATCH 1/4] " Ethan Carter Edwards
2025-03-14 14:38 ` [PATCH 2/4] ASoC: Intel: ssm4567: " Ethan Carter Edwards
2025-03-14 14:38 ` [PATCH 3/4] ASoC: Intel: max98373: " Ethan Carter Edwards
2025-03-14 14:38 ` [PATCH 4/4] ASoC: Intel: max98927: " Ethan Carter Edwards
2025-03-17 10:28 ` Cezary Rojewski [this message]

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=89f83133-eace-4bec-804a-198dee9ae14c@intel.com \
    --to=cezary.rojewski@intel.com \
    --cc=broonie@kernel.org \
    --cc=ethan@ethancedwards.com \
    --cc=gustavoars@kernel.org \
    --cc=kai.vehmanen@linux.intel.com \
    --cc=liam.r.girdwood@linux.intel.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=perex@perex.cz \
    --cc=peter.ujfalusi@linux.intel.com \
    --cc=pierre-louis.bossart@linux.dev \
    --cc=ranjani.sridharan@linux.intel.com \
    --cc=tiwai@suse.com \
    --cc=yung-chuan.liao@linux.intel.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.