Hi Andy, On Thu, 15 Jun 2023 01:05:40 +0300 Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Wed, Jun 14, 2023 at 11:34 PM Herve Codina herve.codina@bootlin.com wrote:
On Wed, 14 Jun 2023 14:51:43 +0300 Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Wed, Jun 14, 2023 at 12:42 PM Herve Codina herve.codina@bootlin.com wrote:
On Wed, 14 Jun 2023 12:02:57 +0300 Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Wed, Jun 14, 2023 at 10:49 AM Herve Codina herve.codina@bootlin.com wrote:
...
typeof(__array[0] + 0) __element = __array[--__len]; \Do we need the ' + 0' part?
Yes.
__array can be an array of const items and it is legitimate to get the minimum value from const items.
typeof(__array[0]) keeps the const qualifier but we need to assign __element in the loop. One way to drop the const qualifier is to get the type from a rvalue computed from __array[0]. This rvalue has to have the exact same type with only the const dropped. '__array[0] + 0' was a perfect canditate.
Seems like this also deserves a comment. But if the series is accepted as is, it may be done as a follow up.
Finally not so simple ... I did some deeper tests and the macros need to be fixed.
I hope this one (with comments added) is correct: --- 8 --- /*
- 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< ---
- The first typeof(&(array)[0]) is needed in order to support arrays of both
- 'int *buff' and 'int buf[N]' types.
- typeof(__array[0] + 0) used for __element is needed as the array can be an
- array of const items.
- In order to discard the const qualifier use an arithmetic operation (rvalue).
- This arithmetic operation discard the const but also can lead to an integer
discards
- promotion. For instance, a const s8 __array[0] lead to an int __element due
leads
- to the promotion.
- In this case, simple min() or max() operation fails (type mismatch).
- Use min_t() or max_t() (op_t parameter) enforcing the type in order to avoid
- the min() or max() failure.
This part perhaps can be avoided. See below.
*/ #define __minmax_array(op_t, array, len) ({ \ typeof(&(array)[0]) __array = (array); \ typeof(len) __len = (len); \ typeof(__array[0] + 0) __element = __array[--__len]; \ while (__len--) \ __element = op_t(typeof(__array[0]), __element, __array[__len]); \
But can't we instead have typeof(+(array[0])) in the definition of __element? There are also other possible solutions: a) _Generic() with listed const types to move them to non-const, and b) __auto_type (which is supported by GCC 4.9 and clang, but not in the C11 standard).
typeof(+(array[0])) keeps the promotion.
__auto_type works with my gcc-12 but not with a gcc-5.5. Depending on the compiler version, it discards or keeps the const qualifier. For this reason I would prefer to not use it.
Did the job using _Generic().
This lead to: --- 8< --- /* * Remove a const qualifier * _Generic(foo, type-name: association, ..., default: association) performs a * comparison against the foo type (not the qualified type). * Do not use the const keyword in the type-name as it will not match the * unqualified type of foo. */ #define __unconst_type_cases(type) \ unsigned type: (unsigned type)0, \ signed type: (signed type)0
#define __unconst_typeof(x) typeof( \ _Generic((x), \ char: (char)0, \ __unconst_type_cases(char), \ __unconst_type_cases(short), \ __unconst_type_cases(int), \ __unconst_type_cases(long), \ __unconst_type_cases(long long), \ default: (x)))
/* * 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< --- * * The first typeof(&(array)[0]) is needed in order to support arrays of both * 'int *buff' and 'int buf[N]' types. * * The array can be an array of const items. * typeof() keeps the const qualifier. Use __unconst_typeof() in order to * discard the const qualifier for the __element variable. */ #define __minmax_array(op, array, len) ({ \ typeof(&(array)[0]) __array = (array); \ typeof(len) __len = (len); \ __unconst_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) --- 8< ---
Do you think it looks good ?
For, the KUnit tests, I agree, it would be nice to have something. I need some more substantial work to implement and run the test in KUnit and the first task will be learning the KUnit test system. I will do that but out of this series.
Thanks for your feedback and pointers, Hervé