linux-kernel - Re: [PATCH v2 5/9] iio: inkern: Add a helper to query an available minimum raw value

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 5 Jun 2023 18:05:47 +0100
From:   Jonathan Cameron <Jonathan.Cameron@...wei.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
CC:     Herve Codina <herve.codina@...tlin.com>,
        Liam Girdwood <lgirdwood@...il.com>,
        Mark Brown <broonie@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Jonathan Cameron <jic23@...nel.org>,
        Lars-Peter Clausen <lars@...afoo.de>,
        Jaroslav Kysela <perex@...ex.cz>,
        Takashi Iwai <tiwai@...e.com>,
        "Kuninori Morimoto" <kuninori.morimoto.gx@...esas.com>,
        <alsa-devel@...a-project.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-iio@...r.kernel.org>,
        Christophe Leroy <christophe.leroy@...roup.eu>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>
Subject: Re: [PATCH v2 5/9] iio: inkern: Add a helper to query an available
 minimum raw value

On Mon, 5 Jun 2023 12:45:24 +0300
Andy Shevchenko <andy.shevchenko@...il.com> wrote:

> On Mon, Jun 5, 2023 at 10:46 AM Herve Codina <herve.codina@...tlin.com> wrote:
> > On Sat, 3 Jun 2023 17:04:37 +0300
> > andy.shevchenko@...il.com wrote:  
> > > Tue, May 23, 2023 at 05:12:19PM +0200, Herve Codina kirjoitti:  
> 
> ...
> 
> > > > +           case IIO_VAL_INT:
> > > > +                   *val = vals[--length];  
> > >  
> > > > +                   while (length) {  
> > >
> > >                       while (length--) {
> > >
> > > will do the job and at the same time...
> > >  
> > > > +                           if (vals[--length] < *val)
> > > > +                                   *val = vals[length];  
> > >
> > > ...this construction becomes less confusing (easier to parse).  
> >
> > Indeed, I will change in the next iteration.  
> 
> And looking into above line, this whole construction I would prefer to
> have a macro in minmax.h like
> 
> #define min_array(array, len) \
> {( \
>   typeof(len) __len = (len); \
>   typeof(*(array)) __element = (array)[--__len]; \
>   while (__len--) \
>     __element = min(__element, (array)[__len]); \
>   __element; \
> )}
> 
> (it might need more work, but you got the idea)
> 
> > > > +                   }
> > > > +                   break;  
> 
> ...
> 
> > > > +           default:
> > > > +                   /* FIXME: learn about min for other iio values */  
> > >
> > > I believe in a final version this comment won't be here.  
> >
> > We have the same FIXME comment in the iio_channel_read_max() function I
> > copied to create this iio_channel_read_min() and, to be honest, I
> > don't really know how to handle these other cases.
> >
> > In this series, I would prefer to keep this FIXME.  
> 
> I see, Jonathan needs to be involved here then.

It's really more of a TODO when someone needs it than a FIXME.
I'm not particularly keen to see a bunch of code supporting cases
that no one uses, but it's useful to call out where the code would
go if other cases were to be supported.

Perhaps soften it to a note that doesn't have the work FIXME in it.

Jonathan


> 
> > > > +                   return -EINVAL;  
>