Re: Coverity: gpmc_is_valid_waitpin(): Control flow issues

[Niedermayr, BENEDIKT]

On Tue, 2022-11-08 at 09:02 +0100, Benedikt Niedermayr wrote:
> On Mon, 2022-11-07 at 10:56 +0200, Roger Quadros wrote:
> > On 07/11/2022 10:53, Roger Quadros wrote:
> > > Hi Benedikt,
> > > 
> > > On 04/11/2022 21:33, coverity-bot wrote:
> > > > Hello!
> > > > 
> > > > This is an experimental semi-automated report about issues detected by
> > > > Coverity from a scan of next-20221104 as part of the linux-next scan project:
> > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fscan.coverity.com%2Fprojects%2Flinux-next-weekly-scan&data=05%7C01%7Cbenedikt.niedermayr%40siemens.com%7C1a25cc8704524f24224108dac09dfab7%7C38ae3bcd95794fd4addab42e1495d55a%7C1%7C0%7C638034081994087461%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=W1KlBKg9nwEDfFAbqW6Jw7v1d46HQLj8RX8wlZ9RHyc%3D&reserved=0
> > > > 
> > > > You're getting this email because you were associated with the identified
> > > > lines of code (noted below) that were touched by commits:
> > > > 
> > > >   Wed Nov 2 10:02:39 2022 -0400
> > > >     89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
> > > > 
> > > > Coverity reported the following:
> > > > 
> > > > *** CID 1527139:  Control flow issues  (NO_EFFECT)
> > > > drivers/memory/omap-gpmc.c:1048 in gpmc_is_valid_waitpin()
> > > > 1042     	spin_unlock(&gpmc_mem_lock);
> > > > 1043     }
> > > > 1044     EXPORT_SYMBOL(gpmc_cs_free);
> > > > 1045
> > > > 1046     static bool gpmc_is_valid_waitpin(u32 waitpin)
> > > 
> > > We will need to change this waitpin argument to int.
> > > In addition we will also need to change
> > > struct gpmc_waitpin->pin and struct gpmc_setting->wait_pin
> > > to int as in the code we are relying on GPMC_WAITPIN_INVALID logic which is -1.
> > 
> > Another alternative with less churn is to leave them as u32
> > but make GPMC_WAITPIN_INVALID set to a large positive number.
> Ok, I will fix that. 
> Do I need to send a new fix-patch on top the current patch series? 
> Or should I just send only the bugfix-patch for the coverity-bot? 
> 
Sorry, another Question: 
Is it somehow possible to check locally if the bugfix actually fixed the bug, before I submit the patch?

> 
> > > > 1047     {
> > > > vvv     CID 1527139:  Control flow issues  (NO_EFFECT)
> > > > vvv     This greater-than-or-equal-to-zero comparison of an unsigned value is always true. "waitpin >= 0U".
> > > > 1048     	return waitpin >= 0 && waitpin < gpmc_nr_waitpins;
> > > > 1049     }
> > > > 1050
> > > > 1051     static int gpmc_alloc_waitpin(struct gpmc_device *gpmc,
> > > > 1052     			      struct gpmc_settings *p)
> > > > 1053     {
> > > > 
> > > > If this is a false positive, please let us know so we can mark it as
> > > > such, or teach the Coverity rules to be smarter. If not, please make
> > > > sure fixes get into linux-next. :) For patches fixing this, please
> > > > include these lines (but double-check the "Fixes" first):
> > > > 
> > > > Reported-by: coverity-bot <keescook+coverity-bot@xxxxxxxxxxxx>
> > > > Addresses-Coverity-ID: 1527139 ("Control flow issues")
> > > > Fixes: 89aed3cd5cb9 ("memory: omap-gpmc: wait pin additions")
> > > > 
> > > > Thanks for your attention!
> > > > 
> > 
> > cheers,
> > -roger
> cheers,
> benedikt
>