Re: [bug report] rtc: Introduce ti-k3-rtc

[Dan Carpenter <dan.carpenter@oracle.com>]

On Thu, Jun 30, 2022 at 08:48:04PM -0500, Nishanth Menon wrote:
> Hi Dan,
> On 09:56-20220628, Dan Carpenter wrote:
> > Hello Nishanth Menon,
> > 
> > The patch b09d633575e5: "rtc: Introduce ti-k3-rtc" from Jun 23, 2022,
> > leads to the following Smatch static checker warning:
> > 
> > 	drivers/rtc/rtc-ti-k3.c:186 k3rtc_unlock_rtc()
> > 	info: return a literal instead of 'ret'
> > 
> > drivers/rtc/rtc-ti-k3.c
> >     180 static int k3rtc_unlock_rtc(struct ti_k3_rtc *priv)
> >     181 {
> >     182         int ret;
> >     183 
> >     184         ret = k3rtc_check_unlocked(priv);
> >     185         if (!ret)
> > --> 186                 return ret;
> > 
> > It look more intentional when code uses literals:
> > 
> > 	if (!ret)
> > 		return 0;
> > 
> > The k3rtc_check_unlocked() function can also return error codes so maybe
> > this should be:
> > 
> > 	if (ret <= 0)
> > 		return 0;
> 
> Interesting for a couple of reasons:
> a) yep - if ret was < 0, driver does'nt anticipate that from regmap
>    op(unless something really bad happened), so it warn_once
>    the information and does continue as if nothing happened.. So, yep, I
>    could potentially do <=0 and document the rationale - might be better
>    to return ret instead of 0 though. I can send a patch if you agree.
> b) How do i reproduce the warning that you reported?

Oh sorry.  I never published this warning.  :/

I generally don't like style checks because life is too short for style
debates.  But the real reason for this check is to find inverted
if (ret) checks.  So maybe I should publish it.  Inverted checks aren't
super common.

regards,
dan carpenter