On 11/10/22 15:06, Dan Carpenter wrote:
> On Thu, Nov 10, 2022 at 05:03:27PM +0300, Dan Carpenter wrote:
>> On Thu, Nov 10, 2022 at 04:11:59PM +0300, Dan Carpenter wrote:
>>> On Thu, Nov 10, 2022 at 12:43:30PM +0100, Benjamin MUGNIER wrote:
>>>> After running smatch on my tree I couldn't reproduce this warning:
>>>> warn: pm_runtime_get_sync() also returns 1 on success
>>>> I'm using the latest smatch cloned from github. Do you append some
>>>> options to kchecker to get this output ?
>>>
>>> TL;DR: Thanks for the report! I will fix it later this week.
>>>
>>
>> [ snip ]
>>
>>> It creates a fake environment to test what !ret means
>>> for uninitialized variables. The check_pm_runtime_get_sync.c check sees
>>> the "!ret" condition and says, "Nope. That's supposed to be "ret < 0"".
>>>
>>> Smatch shouldn't be printing warnings from inside the fake environment.
>>
>> Nope. That's not it... It already has code to not print from a fake
>> environment (unless you're in debug mode). It's a mystery how the
>> kbuild bot triggered this warning.
>>
>> :(
>
> Ah... Seeing your patch helped me figure it out. The kbuild bot does
> not have the cross function db built so when it sees:
>
> vgxy61_write_reg(sensor, VGXY61_REG_ROI0_START_V, crop->top, &ret);
>
> Then it doesn't see that "ret" is modified. On my system I have the
> DB so I don't see the warning.
I also have the DB, so that explains why.
However, 'vgxy61_write_reg' does not always modify 'ret'. In fact it's
worse, at the beginning it checks if it not 0 in 'vgxy61_write_multiple'
and returns if it's not:
if (err && *err)
return *err;
with 'err' being an alias to 'ret'.
I did this to avoid checking all my i2c writes. I just send a bunch of
them and check the return code at the end. But if one i2c write fails,
'err' is filled and all following writes are aborted due to the code above.
To summarize: if 'pm_runtime_get_sync' ever returned 1, no i2c write was
performed. That's why I needed to set 'ret' to 0 in my patch.
The warning spared me from debugging this, thanks a lot.
Could this warning be produced even with having the DB?
>
> regards,
> dan carpenter
>
--
Regards,
Benjamin
