Re: [PATCH 1/2] selftests: Provide local define of min() and max()

From: Mahmoud Matook
Date: Wed Aug 23 2023 - 15:36:52 EST


On 08/22, David Laight wrote:

> ...
> > > That typecheck() is horrid.
> > > It may well have caused more bugs due to incorrect casts that
> > > it actually detected.
> > >
> > > I'd suggest the version that just avoids multiple evaluations.
> > > Or just error signed v unsigned comparisons.
> > > See https://lore.kernel.org/all/b4ce9dad748e489f9314a2dc95615033@xxxxxxxxxxxxxxxx/
> > > for an example patch set.
> >
> > Interesting, thanks. That is also simpler.
> >
> > Also, the existing patch is no worse than the open coded code today,
> > so even without code to avoid multiple evaluations, I guess it's okay
> > to merge.
> >
> > The coccinelle warnings are arguably false positives, using checks for
> > kernel code, but being run against userspace code that has no access
> > to those helpers. But fine to silence them.
>
> You can't use is_constexpr() unless 'sizeof *(void *)' is valid.
> And builtin_constant() isn't good enough for builtin_choose_expr().
>
> That might be ok for selftests and tools, but not for generaluserspace.
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> Registration No: 1397386 (Wales)
I tried to use the relaxed version provided in the shared patchset link
besides not able to use is_constexpr(), I'm not able to use
__UNIQUE_ID() also. It's definded inside include/linux/compiler-gcc.h
and it uses another macro __PASTE() which is defined inside
include/linux/compiler_types.h.
not sure what to do next
- bring those macros definitions to able to use the relaxed version.
- if the most important point for min/max defines inside selftests is to
avoid multiple evaluation is the below version acceptable?
/*
#define min(x, y) ({ \
typeof(x) _x = (x); \
typeof(y) _y = (y); \
_x < _y ? _x : _y; \
})

#define max(x, y) ({ \
typeof(x) _x = (x); \
typeof(y) _y = (y); \
_x > _y ? _x : _y; \
})
*/