linux-kernel - RE: [PATCH v4 1/2] compiler.h: add const

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]

Message-ID: <2b5282038b1f46bc9a658fb2b6d78350@AcuMS.aculab.com>
Date: Sun, 17 Nov 2024 22:38:19 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Linus Torvalds' <torvalds@...ux-foundation.org>
CC: Vincent Mailhol <mailhol.vincent@...adoo.fr>, Yury Norov
	<yury.norov@...il.com>, Rasmus Villemoes <linux@...musvillemoes.dk>, "Luc Van
 Oostenryck" <luc.vanoostenryck@...il.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-sparse@...r.kernel.org"
	<linux-sparse@...r.kernel.org>, Rikard Falkeborn <rikard.falkeborn@...il.com>
Subject: RE: [PATCH v4 1/2] compiler.h: add const_true()

From: Linus Torvalds
> Sent: 17 November 2024 20:12
> 
> On Sun, 17 Nov 2024 at 11:23, David Laight <David.Laight@...lab.com> wrote:
> >
> > Since 99% will be 1,0 maybe saving the extra expansion is best anyway.
> > So have is_const_zero(x) and add if_const_zero(x, if_z, if_nz) later.
> 
> Ok. So something like this seems to give us the relevant cases:
> 
>   #define __is_const_zero(x) \
>         _Generic(0?(void *)(long)(x):(char *)0, char *:1, void *:0)
> 
>   #define is_const_zero(x) __is_const_zero(!!(x))
>   #define is_const_true(x) __is_const_zero(!(x))
>   #define is_const(x) __is_const_zero(0*!(x))
> 
> and should work with all scalar expressions that I can think of (ok,
> technically 'void' is a scalar type and it obviously won't work with
> that). And should work in all contexts.

Seems a reasonable set.

Maybe they need a set that are paired with __BUILD_BUG_ON_ZERO_MSG()
to generate an error message on failure.

Although I would add a few more ' ' characters for readability.

> It does want a comment (in addition to the comment about how NULL is
> special for the ternary op that makes it work): the '(long)' cast is
> so that there are no warnings for casting to 'void *' when it's *not*
> a constant zero expression, and the '!' pattern is to turn pointers
> and huge constants into 'int' without loss of information and without
> warnings.

The comments would need to be terse one-liners.

I wonder if it reads better (and without extra comments) if the (long)
cast is removed and the 'callers' are required to generate 'long' args.
So you have:

#define __is_const_zero(x) \
	_Generic(0 ? (void *)(x) : (char *)0, char *: 1, void *: 0)
 
#define is_const_zero(x) __is_const_zero((x) ? 1L : 0L)
#define is_const_true(x) __is_const_zero((x) ? 0L : 1L)
#define is_const(x) __is_const_zero((x) ? 0L : 0L)

I've done a quick test of the last one in godbolt.

	David

> 
> Compound types obviously will generate a warning. As they should.
> 
> The above looks reasonable to me, but I didn't actually test any of it
> in the actual kernel build.
> 
>              Linus

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)