Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1720663 > unrolled thread

[PATCH] Staging rtlwifi efuse fix up a warning kzalloc

Back to article view | Back to linux.kernel

Contents

  [PATCH] Staging rtlwifi efuse fix up a warning kzalloc Yurii Pavlenko <pyldev@gmail.com> - 2017-08-26 15:10 +0200
    Re: [PATCH] Staging rtlwifi efuse fix up a warning kzalloc Larry Finger <Larry.Finger@lwfinger.net> - 2017-08-26 16:50 +0200
      Re: [PATCH] Staging rtlwifi efuse fix up a warning kzalloc Joe Perches <joe@perches.com> - 2017-08-26 17:30 +0200
        Re: [PATCH] Staging rtlwifi efuse fix up a warning kzalloc Dan Carpenter <dan.carpenter@oracle.com> - 2017-08-28 10:40 +0200

#1720663 — [PATCH] Staging rtlwifi efuse fix up a warning kzalloc

Hello,

I have attached a small patch to fix a warning "Prefer kcalloc over kzalloc with multiply"
for efuse.c as part of challenge 10 of Eudyptula. 

Best regards,
Yurii Pavlenko


Signed-off-by: Yurii Pavlenko <pyldev@gmail.com>
---
 drivers/staging/rtlwifi/efuse.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rtlwifi/efuse.c b/drivers/staging/rtlwifi/efuse.c
index 6d5e657..520d4f2 100644
--- a/drivers/staging/rtlwifi/efuse.c
+++ b/drivers/staging/rtlwifi/efuse.c
@@ -248,15 +248,15 @@ void read_efuse(struct ieee80211_hw *hw, u16 _offset, u16 _size_byte, u8 *pbuf)
 	}
 
 	/* allocate memory for efuse_tbl and efuse_word */
-	efuse_tbl = kzalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE] *
+	efuse_tbl = kcalloc(rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE],
 			    sizeof(u8), GFP_ATOMIC);
 	if (!efuse_tbl)
 		return;
-	efuse_word = kzalloc(EFUSE_MAX_WORD_UNIT * sizeof(u16 *), GFP_ATOMIC);
+	efuse_word = kcalloc(EFUSE_MAX_WORD_UNIT, sizeof(u16 *), GFP_ATOMIC);
 	if (!efuse_word)
 		goto out;
 	for (i = 0; i < EFUSE_MAX_WORD_UNIT; i++) {
-		efuse_word[i] = kzalloc(efuse_max_section * sizeof(u16),
+		efuse_word[i] = kcalloc(efuse_max_section, sizeof(u16),
 					GFP_ATOMIC);
 		if (!efuse_word[i])
 			goto done;
-- 
2.7.4

[toc] | [next] | [standalone]

#1720670

On 08/26/2017 08:01 AM, Yurii Pavlenko wrote:
> Hello,
> 
> I have attached a small patch to fix a warning "Prefer kcalloc over kzalloc with multiply"
> for efuse.c as part of challenge 10 of Eudyptula.
> 
> Best regards,
> Yurii Pavlenko
> 
> 
> Signed-off-by: Yurii Pavlenko <pyldev@gmail.com>

Before you waste any more of our time, please read the material about patch 
submission at Documentation/process/submitting-patches.rst in your source tree. 
In its present form, this patch is not usable!

I also disagree with the checkpatch warning. To me, there is no difference 
between specifying the size of the allocation as "EFUSE_MAX_WORD_UNIT, 
sizeof(u16 *)" or "EFUSE_MAX_WORD_UNIT * sizeof(u16 *)". In  fact, the only real 
difference is that the source is ONE character larger with the kzalloc version! 
Is that important? Certainly not to me! One thing that is readily apparent is 
that kzalloc() zeros the allocated space. Of course, so does kcalloc(), but it 
is not apparent from the name.

Most of the checkpatch warnings improve readability of the source, and do find 
real or potential errors. This particular one does not, and I will NACK every 
patch that tries to force code that I maintain to use kcalloc over kzalloc.

Larry

[toc] | [prev] | [next] | [standalone]

#1720677

On Sat, 2017-08-26 at 09:48 -0500, Larry Finger wrote:
> I will NACK every 
> patch that tries to force code that I maintain to use kcalloc over kzalloc.

Your choice.

The concept behind the kzalloc with multiply vs kcalloc
is protection against multiplication overflow.

It's unlikely to solve any real problem with a
sizeof(foo) * small known number of entries.

It does have application when number of entries is
a user specified value.

[toc] | [prev] | [next] | [standalone]

#1721290

Yeah...  I reviewed this code to see where
rtlpriv->cfg->maps[EFUSE_HWSET_MAX_SIZE] gets set and if it can be
controlled by the user.

It looks to me like it's only set when we declare rtl8822be_hal_cfg in
drivers/staging/rtlwifi/rtl8822be/sw.c so it's fine. I think it should
probably be a big struct instead of an array.

regards,
dan carpenter

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web