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
