Re: [SPF Softfail] Re: [PATCH v7 03/10] lib: add linear ranges helpers

From: Andy Shevchenko <andy.shevchenko@gmail.com>
To: "Vaittinen, Matti" <Matti.Vaittinen@fi.rohmeurope.com>
Cc: "mark.rutland@arm.com" <mark.rutland@arm.com>,
	"milo.kim@ti.com" <milo.kim@ti.com>,
	"andrei.stefanescu@microchip.com"
	<andrei.stefanescu@microchip.com>,
	"rafael@kernel.org" <rafael@kernel.org>,
	"tony@atomide.com" <tony@atomide.com>,
	"linus.walleij@linaro.org" <linus.walleij@linaro.org>,
	"brendanhiggins@google.com" <brendanhiggins@google.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"yamada.masahiro@socionext.com" <yamada.masahiro@socionext.com>,
	"linux-omap@vger.kernel.org" <linux-omap@vger.kernel.org>,
	"vincenzo.frascino@arm.com" <vincenzo.frascino@arm.com>,
	"tglx@linutronix.de" <tglx@linutronix.de>,
	"linux-samsung-soc@vger.kernel.org"
	<linux-samsung-soc@vger.kernel.org>,
	"herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
	"b.zolnierkie@samsung.com" <b.zolnierkie@samsung.com>,
	"krzk@kernel.org" <krzk@kernel.org>,
	"bgolaszewski@baylibre.com" <bgolaszewski@baylibre.com>,
	"wens@csie.org" <wens@csie.org>,
	"agross@kernel.org" <agross@kernel.org>,
	"Laine, Markus" <Markus.Laine@fi.rohmeurope.com>,
	"bp@suse.de" <bp@suse.de>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"zaslonko@linux.ibm.com" <zaslonko@linux.ibm.com>,
	"ckeepax@opensource.cirrus.com" <ckeepax@opensource.cirrus.com>,
	"arnd@arndb.de" <arnd@arndb.de>,
	"linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
	"linux-arm-msm@vger.kernel.org" <linux-arm-msm@vger.kernel.org>,
	"rf@opensource.cirrus.com" <rf@opensource.cirrus.com>,
	"broonie@kernel.org" <broonie@kernel.org>,
	"linux-mediatek@lists.infradead.org"
	<linux-mediatek@lists.infradead.org>,
	"sre@kernel.org" <sre@kernel.org>,
	"davidgow@google.com" <davidgow@google.com>,
	"skhan@linuxfoundation.org" <skhan@linuxfoundation.org>,
	"matthias.bgg@gmail.com" <matthias.bgg@gmail.com>,
	"dan.j.williams@intel.com" <dan.j.williams@intel.com>,
	"bjorn.andersson@linaro.org" <bjorn.andersson@linaro.org>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"support.opensource@diasemi.com" <support.opensource@diasemi.com>,
	"sbkim73@samsung.com" <sbkim73@samsung.com>,
	"patches@opensource.cirrus.com" <patches@opensource.cirrus.com>,
	"rdunlap@infradead.org" <rdunlap@infradead.org>,
	"lgirdwood@gmail.com" <lgirdwood@gmail.com>,
	"talgi@mellanox.com" <talgi@mellanox.com>,
	"Mutanen, Mikko" <Mikko.Mutanen@fi.rohmeurope.com>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"uwe@kleine-koenig.org" <uwe@kleine-koenig.org>,
	"mhiramat@kernel.org" <mhiramat@kernel.org>,
	"gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
	"akpm@linux-foundation.org" <akpm@linux-foundation.org>,
	"olteanv@gmail.com" <olteanv@gmail.com>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"mazziesaccount@gmail.com" <mazziesaccount@gmail.com>
Subject: Re: [SPF Softfail] Re: [PATCH v7 03/10] lib: add linear ranges helpers
Date: Wed, 1 Apr 2020 17:42:31 +0300	[thread overview]
Message-ID: <CAHp75Vd24w6dqqCYH46GKnJ5Nzase6LdVgSSVwQ3FEzMd5gsYA@mail.gmail.com> (raw)
In-Reply-To: <e2391c0749478d42ac7e2db4981b9d373a160323.camel@fi.rohmeurope.com>

On Wed, Apr 1, 2020 at 4:18 PM Vaittinen, Matti
<Matti.Vaittinen@fi.rohmeurope.com> wrote:
> On Wed, 2020-04-01 at 15:39 +0300, Andy Shevchenko wrote:
> > On Wed, Apr 1, 2020 at 1:09 PM Vaittinen, Matti
> > <Matti.Vaittinen@fi.rohmeurope.com> wrote:
> > > On Tue, 2020-03-31 at 17:05 +0300, Andy Shevchenko wrote:
> > > > On Tue, Mar 31, 2020 at 03:23:03PM +0300, Matti Vaittinen wrote:
> > > > > +int linear_range_get_selector_low_array(const struct
> > > > > linear_range
> > > > > *r,
> > > > > +                                   int ranges, unsigned int
> > > > > val,
> > > > > +                                   unsigned int *selector,
> > > > > bool
> > > > > *found)
> > > > > +{
> > > > > +   int i;
> > > > > +   int ret = -EINVAL;
> > > > > +
> > > > > +   for (i = 0; i < ranges; i++) {
> > > > > +           int tmpret;
> > > > > +
> > > > > +           tmpret = linear_range_get_selector_low(&r[i], val,
> > > > > selector,
> > > > > +                                                  found);
> > > > > +
> > > > > +           if (!tmpret)
> > > > > +                   ret = 0;
> > > > > +
> > > > > +           if (*found)
> > > > > +                   break;
> > > > > +   }
> > > > > +
> > > > > +   return ret;
> > > > > +}

Looked again at the code of the callee.
So, *found becomes true if and only if the return is 0 (or other way
around if you prefer).
Now I'm wondering why you need 'found' at all?

It means above may be as simple as

      int i;
      int ret = -EINVAL;

      for (i = 0; i < ranges; i++) {
               ret = linear_range_get_selector_low(&r[i], val, selector, found);
               if (*found)
                       break;
      }

      return ret;

or assuming 'found' will gone

  int i
  int ret = -EINVAL;

  for (i = 0; i < ranges && ret; i++) {
      ret = linear_range_get_selector_low(&r[i], val, selector);
  }
  return ret;

...

>  * value. If given value is found to be in a range scannins is

> I think it is but I am open to all suggestions how to improve doc!

Thanks. At least fix a typo: scannins -> scannings

...

> Compared to:
> for (i = 0; i < ranges; i++) {
>         ret = ...
> }
>
> I wouldn't say so.
>
> As I explained, we need to have "temporary" return value in any case
> because we need to return 0 if any of the calls to
> linear_range_get_selector_low() returned 0. Return value 0 from
> linear_range_get_selector_low() means we found "matching" value (lower
> than input) and selector was updated (although input value was not in
> range).

See above.

--
With Best Regards,
Andy Shevchenko

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek