| Message ID | 20230612122926.107333-7-herve.codina@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for IIO devices in ASoC | expand |
Hi Andy, On Mon, 12 Jun 2023 17:10:40 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <herve.codina@bootlin.com> wrote: > > > > Introduce min_array() (resp max_array()) in order to get the > > minimal (resp maximum) of values present in an array. > > Some comments below, after addressing them, > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > > Signed-off-by: Herve Codina <herve.codina@bootlin.com> > > --- > > include/linux/minmax.h | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > > > diff --git a/include/linux/minmax.h b/include/linux/minmax.h > > index 396df1121bff..37a211f22404 100644 > > --- a/include/linux/minmax.h > > +++ b/include/linux/minmax.h > > @@ -133,6 +133,32 @@ > > */ > > #define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >) > > > > +#define __minmax_array(op, array, len) ({ \ > > Maybe it's my MUA, maybe the code contains spaces, can you switch to > TABs if it's the case? > > > + typeof(array) __array = (array); \ > > We have __must_be_array() Using __must_be_array() will lead to some failure. Indeed, we can have: --- 8< --- int *buff ... min = min_array(buff, nb_item); --- 8< --- In this case, __must_be_array() will report that buff is not an array. To avoid any confusion, what do you think if I renamed {min,max}_array() to {min,max}_buffer() and replace __array by __buff and use *(__buff + xxx) instead of array[xxx] in the macro. This will lead to: --- 8< --- #define __minmax_buffer(op, buff, len) ({ \ typeof(buff) __buff = (buff); \ typeof(len) __len = (len); \ typeof(*buff + 0) __element = *(__buff + --__len); \ while (__len--) \ __element = op(__element, *(__buff + __len])); \ __element; }) #define min_buffer(buffer, len) __minmax_array(min, buffer, len) #define max_buffer(buffer, len) __minmax_array(max, buffer, len) --- 8< --- Regards, Hervé > > You will need to fix the inclusions in minmax.h at the same time, it needs > linux/build_bug.h (which includes compiler.h needed for __UNIQUE_ID() > and for the above mentioned one). > > > + typeof(len) __len = (len); \ > > + typeof(*__array + 0) __element = __array[--__len]; \ > > After above, this can be written as __array[0]. > > > + while (__len--) \ > > + __element = op(__element, __array[__len]); \ > > + __element; }) > > + > > +/** > > + * min_array - return minimum of values present in an array > > + * @array: array > > + * @len: array length > > + * > > + * Note that @len must not be zero (empty array). > > + */ > > +#define min_array(array, len) __minmax_array(min, array, len) > > + > > +/** > > + * max_array - return maximum of values present in an array > > + * @array: array > > + * @len: array length > > + * > > + * Note that @len must not be zero (empty array). > > + */ > > +#define max_array(array, len) __minmax_array(max, array, len) > > + > > /** > > * clamp_t - return a value clamped to a given range using a given type > > * @type: the type of variable to use >
On Tue, Jun 13, 2023 at 11:00 AM Herve Codina <herve.codina@bootlin.com> wrote: > On Mon, 12 Jun 2023 17:10:40 +0300 > Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <herve.codina@bootlin.com> wrote: > > > > > > Introduce min_array() (resp max_array()) in order to get the > > > minimal (resp maximum) of values present in an array. > > > > Some comments below, after addressing them, > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> ... > > > + typeof(array) __array = (array); \ > > > > We have __must_be_array() > > Using __must_be_array() will lead to some failure. > Indeed, we can have: > --- 8< --- > int *buff > ... > min = min_array(buff, nb_item); > --- 8< --- > > In this case, __must_be_array() will report that buff is not an array. Oh, I missed that. > To avoid any confusion, what do you think if I renamed {min,max}_array() > to {min,max}_buffer() and replace __array by __buff and use *(__buff + xxx) > instead of array[xxx] in the macro. But functionally it's still against an array. I would stick with "array" in the name, but add a comment why __must_be_array() is not used. If we need a stricter variant, we may add a new wrapper with that check. That said, I think we can use __array[0] and similar indexed accesses. -- With Best Regards, Andy Shevchenko
On Tue, 13 Jun 2023 20:08:08 +0300 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Tue, Jun 13, 2023 at 11:00 AM Herve Codina <herve.codina@bootlin.com> wrote: > > On Mon, 12 Jun 2023 17:10:40 +0300 > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > > > On Mon, Jun 12, 2023 at 3:30 PM Herve Codina <herve.codina@bootlin.com> wrote: > > > > > > > > Introduce min_array() (resp max_array()) in order to get the > > > > minimal (resp maximum) of values present in an array. > > > > > > Some comments below, after addressing them, > > > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com> > > ... > > > > > + typeof(array) __array = (array); \ > > > > > > We have __must_be_array() > > > > Using __must_be_array() will lead to some failure. > > Indeed, we can have: > > --- 8< --- > > int *buff > > ... > > min = min_array(buff, nb_item); > > --- 8< --- > > > > In this case, __must_be_array() will report that buff is not an array. > > Oh, I missed that. > > > To avoid any confusion, what do you think if I renamed {min,max}_array() > > to {min,max}_buffer() and replace __array by __buff and use *(__buff + xxx) > > instead of array[xxx] in the macro. > > But functionally it's still against an array. > > I would stick with "array" in the name, but add a comment why > __must_be_array() is not used. If we need a stricter variant, we may > add a new wrapper with that check. That said, I think we can use > __array[0] and similar indexed accesses. > Right, I will provide an updated version on the next iteration. Thanks for your feedback. Hervé
diff --git a/include/linux/minmax.h b/include/linux/minmax.h index 396df1121bff..37a211f22404 100644 --- a/include/linux/minmax.h +++ b/include/linux/minmax.h @@ -133,6 +133,32 @@ */ #define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >) +#define __minmax_array(op, array, len) ({ \ + typeof(array) __array = (array); \ + typeof(len) __len = (len); \ + typeof(*__array + 0) __element = __array[--__len]; \ + while (__len--) \ + __element = op(__element, __array[__len]); \ + __element; }) + +/** + * min_array - return minimum of values present in an array + * @array: array + * @len: array length + * + * Note that @len must not be zero (empty array). + */ +#define min_array(array, len) __minmax_array(min, array, len) + +/** + * max_array - return maximum of values present in an array + * @array: array + * @len: array length + * + * Note that @len must not be zero (empty array). + */ +#define max_array(array, len) __minmax_array(max, array, len) + /** * clamp_t - return a value clamped to a given range using a given type * @type: the type of variable to use
Introduce min_array() (resp max_array()) in order to get the minimal (resp maximum) of values present in an array. Signed-off-by: Herve Codina <herve.codina@bootlin.com> --- include/linux/minmax.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)