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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

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; \
})
*/