| Message ID | 20230614074904.29085-8-herve.codina@bootlin.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for IIO devices in ASoC | expand |
Hi David, On Fri, 16 Jun 2023 09:08:22 +0000 David Laight <David.Laight@ACULAB.COM> wrote: ... > > Just define two variables typeof(__array[0] + 0) one for an element > and one for the limit. > The just test (eg): > if (limit > item) limit = item; > finally cast the limit back to the original type. > The promotions of char/short to signed int won't matter. > There is no need for all the type-checking in min/max. > > Indeed, if min_t(type, a, b) is in anyway sane it should > expand to: > type _a = a, _b = b; > _a < _b ? _a : _b > without any of the checks that min() does. I finally move to use _Generic() in order to "unconstify" and avoid the integer promotion. With this done, no extra cast is needed and min()/max() are usable. The patch is available in the v5 series. https://lore.kernel.org/linux-kernel/20230615152631.224529-8-herve.codina@bootlin.com/ Do you think the code present in the v5 series should be changed ? If so, can you give me your feedback on the v5 series ? Thanks for your review, Hervé
From: Herve Codina <herve.codina@bootlin.com> > Sent: 16 June 2023 12:49 > > Hi David, > > On Fri, 16 Jun 2023 09:08:22 +0000 > David Laight <David.Laight@ACULAB.COM> wrote: > > ... > > > > > Just define two variables typeof(__array[0] + 0) one for an element > > and one for the limit. > > The just test (eg): > > if (limit > item) limit = item; > > finally cast the limit back to the original type. > > The promotions of char/short to signed int won't matter. > > There is no need for all the type-checking in min/max. > > > > Indeed, if min_t(type, a, b) is in anyway sane it should > > expand to: > > type _a = a, _b = b; > > _a < _b ? _a : _b > > without any of the checks that min() does. > > I finally move to use _Generic() in order to "unconstify" and avoid the > integer promotion. With this done, no extra cast is needed and min()/max() > are usable. > > The patch is available in the v5 series. > https://lore.kernel.org/linux-kernel/20230615152631.224529-8-herve.codina@bootlin.com/ > > Do you think the code present in the v5 series should be changed ? > If so, can you give me your feedback on the v5 series ? It seems horribly over-complicated just to get around the perverse over-strong type checking that min/max do just to avoid sign extension issues. Maybe I ought to try getting a patch accepted that just checks is_signed_type(typeof(x)) == is_signed_type(typeof(y)) instead of typeof(x) == typeof(y) Then worry about the valid signed v unsigned cases. Indeed, since the array index can be assumed not to have side effects you could use __cmp(x, y, op) directly. No one has pointed out that __element should be __bound. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
diff --git a/include/linux/minmax.h b/include/linux/minmax.h index 396df1121bff..2cd0d34ce921 100644 --- a/include/linux/minmax.h +++ b/include/linux/minmax.h @@ -133,6 +133,42 @@ */ #define max_t(type, x, y) __careful_cmp((type)(x), (type)(y), >) +/* + * Do not check the array parameter using __must_be_array(). + * In the following legit use-case where the "array" passed is a simple pointer, + * __must_be_array() will return a failure. + * --- 8< --- + * int *buff + * ... + * min = min_array(buff, nb_items); + * --- 8< --- + */ +#define __minmax_array(op, array, len) ({ \ + typeof(array) __array = (array); \ + typeof(len) __len = (len); \ + typeof(__array[0] + 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 | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+)