[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230823193545.nrzlbsa32hm4os4k@mmaatuq-HP-Laptop-15-dy2xxx>
Date: Wed, 23 Aug 2023 23:35:45 +0400
From: Mahmoud Matook <mahmoudmatook.mm@...il.com>
To: David Laight <David.Laight@...LAB.COM>
Cc: 'Willem de Bruijn' <willemdebruijn.kernel@...il.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-kselftest@...r.kernel.org" <linux-kselftest@...r.kernel.org>,
"kuba@...nel.org" <kuba@...nel.org>,
"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"davem@...emloft.net" <davem@...emloft.net>,
"pabeni@...hat.com" <pabeni@...hat.com>,
"edumazet@...gle.com" <edumazet@...gle.com>,
"shuah@...nel.org" <shuah@...nel.org>,
"linux-kernel-mentees@...ts.linuxfoundation.org" <linux-kernel-mentees@...ts.linuxfoundation.org>
Subject: Re: [PATCH 1/2] selftests: Provide local define of min() and max()
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@AcuMS.aculab.com/
> > > 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; \
})
*/
Powered by blists - more mailing lists