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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Dan,

I'm sorry. There was an opinion that the pach sent last time was inconvenient to look at because the entire patch aws modified at once.

I just misunderstood the patch system, so I re-edited it and sent it again.
I'll continue to use this patch later.
Thanks for your kindness. I'll get your description below.


-----Original Message-----
> Subject: Re: [PATCH 14/14] Fixed the retry_cnt bug about being zero

> There are a number of issues with this patch...  :(

> The bug reports were from kbuild so you'll probably need to just resend the patch series from before as a v2 or something.  It can't be [PATCH 14/14].  Where are the others in the series?

> If you do fix these issues as a separate patch:
> 1) It needs a subsystem prefix like "[PATCH] ASoC: sma1303: " or something.
> 2) It fixes 3 different issues so it should be 3 different patches.
> 3) It needs a commit description.
> 4) It needs a Fixes tag.

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

>>  
>>  	for (index = 0; index < ARRAY_SIZE(sma1303_snd_controls); index++) {
>>  		sma1303_controls[index] = sma1303_snd_controls[index];
>>  		name[index] = devm_kzalloc(sma1303->dev,
>> -				MAX_CONTROL_NAME, GFP_KERNEL);
>> +				MAX_CONTROL_NAME*sizeof(char), GFP_KERNEL);

>sizeof(char) is not required.  It's always zero.

>>  		size = strlen(sma1303_snd_controls[index].name)
>>  			+ strlen(sma1303->dev->driver->name);
>>  		if (!name[index] || size > MAX_CONTROL_NAME) {

regards,
dan carpenter





[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux