Re: [bug report] net: ipa: reduce arguments to ipa_table_init_add()

From: Alex Elder <alex.elder@linaro.org>
To: Dan Carpenter <error27@gmail.com>
Cc: elder@linaro.org, kernel-janitors@vger.kernel.org
Subject: Re: [bug report] net: ipa: reduce arguments to ipa_table_init_add()
Date: Sat, 19 Nov 2022 03:04:50 -0600	[thread overview]
Message-ID: <847759a7-9501-194a-0314-2ec3eceb1613@linaro.org> (raw)
In-Reply-To: <Y3W83708rvg1Krvy@kadam>

On 11/16/22 22:47, Dan Carpenter wrote:
> On Tue, Nov 15, 2022 at 11:00:49AM -0600, Alex Elder wrote:
>>> drivers/net/ipa/ipa_table.c
>>>      413			count = mem->size / sizeof(__le64);
>>>      414			hash_count = hash_mem && hash_mem->size / sizeof(__le64);
>>
>> Line 414 is wrong.  It should be:
>>      hash_count = hash_mem ? hash_mem->size / sizeof(__le64) : 0;
>>
> 
> Heh.  It really feels like this line should have generated a checker
> warning as well.  I've created two versions.  The first complains when
> ever there is a divide used as a condition:

I agree, this is a good case to identify if you can.

> 
> 	if (a / b) {

That seems reasonable.  In a real "if" statement it is
along the lines of an assignment used as a condition,
where a second set of parentheses can be used to indicate
it's intentional.

> The other complains when it's part of a logical && or ||.
> 
> 	if (a && a / b) {

I don't know how Smatch works, but at the C parser level
it seems any divide used in a conditional context might
be worth paying attention to.

> 
> drivers/net/ipa/ipa_table.c:414 ipa_table_init_add() warn: divide condition: 'hash_mem->size / 8'
> drivers/net/ipa/ipa_table.c:414 ipa_table_init_add() warn: divide condition (logical): 'hash_mem->size / 8'
> 
> I'll test them out tonight and see if either gives useful results.

Sounds good.  Sorry I didn't respond a few days ago, I'm away
from home and am not keeping up with everything well.

					-Alex

> 
> regards,
> dan carpenter
> 