Re: [PATCH 14/14] Fixed the retry_cnt bug about being zero

[Dan Carpenter <error27@gmail.com>]

On Fri, Jan 06, 2023 at 09:55:43AM +0000, Ki-Seok Jo wrote:
> >> @@ -772,12 +772,13 @@ static int sma1303_add_component_controls(struct snd_soc_component *component)
> >>  	sma1303_controls = devm_kzalloc(sma1303->dev,
> >>  			sizeof(sma1303_snd_controls), GFP_KERNEL);
> >>  	name = devm_kzalloc(sma1303->dev,
> >> -			ARRAY_SIZE(sma1303_snd_controls), GFP_KERNEL);
> >> +			ARRAY_SIZE(sma1303_snd_controls)*sizeof(char *),
> >> +			GFP_KERNEL);
> 
> >I am surprised checkpatch doesn't complain that spaces are required around the * operator.  Please just use sizeof(sma1303_snd_controls).
> Otherwise you have to use devm_kcalloc() to avoid checkers warning about integer overflows.
> 
> I lost space between * operator. Thanks. (why didn't checkpatch check it? :(  )
> 
> But I don't understand why I use 'sizeof(sma1303_snd_controls)'.
> I only need to know the number of 'sma1303_snd_controls'.
> In 'sma1303_snd_controls', it has only 3.
> 
> So ARRAY_SIZE(sma1303_snd_controls) is 3, but sizeof(sma1303_snd_controls) has the value of 144.
> I think it's not necessary. What's the best?
> 

Ah.  Sorry, I didn't have enough context.  But could you instead use
sizeof(*name) instead of (char *) (it's the standard kernel style and
not just my opinion):

	name = devm_kcalloc(sma1303->dev, ARRAY_SIZE(sma1303_snd_controls),
			    sizeof(*name), GFP_KERNEL);

Also please declare name as char instead of unsigned char.
Also there needs to be some error checking for if the allocation fails.

This driver is going to need quite a bit of cleanup.  :/

regards,
dan carpenter