linux-kernel - RE: [PATCH v2 1/8] minmax: Put all the clamp() definitions together

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: <fbdf2df07e7140dbb96dcda48d4da270@AcuMS.aculab.com>
Date: Wed, 31 Jul 2024 15:56:15 +0000
From: David Laight <David.Laight@...LAB.COM>
To: 'Linus Torvalds' <torvalds@...uxfoundation.org>
CC: Arnd Bergmann <arnd@...nel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, Jens Axboe <axboe@...nel.dk>, Matthew Wilcox
	<willy@...radead.org>, Christoph Hellwig <hch@...radead.org>, Andrew Morton
	<akpm@...ux-foundation.org>, Andy Shevchenko
	<andriy.shevchenko@...ux.intel.com>, Dan Carpenter
	<dan.carpenter@...aro.org>, "Jason A . Donenfeld" <Jason@...c4.com>,
	"pedro.falcato@...il.com" <pedro.falcato@...il.com>, Mateusz Guzik
	<mjguzik@...il.com>, "linux-mm@...ck.org" <linux-mm@...ck.org>, "Lorenzo
 Stoakes" <lorenzo.stoakes@...cle.com>
Subject: RE: [PATCH v2 1/8] minmax: Put all the clamp() definitions together

From: Linus Torvalds
> Sent: 31 July 2024 16:38
> On Wed, 31 Jul 2024 at 01:10, David Laight <David.Laight@...lab.com> wrote:
> >
> > The __UNIQUE_ID_() define just seemed excessive - especially
> > since all compiler versions support __COUNTER__.
> 
> Yes, we could probably just simplify it.
...
> So that "prefix" literally exists because it literally wasn't unique
> enough without it.

I don't remember that far back :-)

> And the "__UNIQUE_ID_" thing is probably because that way it was
> clearer what was going on when something went wrong.
> 
> But together they really end up being a somewhat unreadable mess.
> 
> That said, I did end up liking the "prefix" part when looking at
> expansions, because it helps show "which" expansion it is (ie "x_123"
> and "y_124" were clearer than just some pure counter value that
> doesn't have any relationship to the origin at all in the name).

I guess that once the caller-supplied prefix was added the fixed
__UNIQUE_ID_ bit could just have been removed.

> But I did change it to "x_" from "__x", because that way it was
> minimal, yet clearly separate from the counter number (ie "x_123" was
> better than "__x123").

Indeed...

I got annoyed because the unique 'x' and 'y' for min() end up
having differ numbers - which can make it harder see what is going
on with nested expansions.
I might even have done a global replace to get rid of the __UNIQUE_ID_
text in an attempt to make the expansions readable.

Perhaps something like:
#define do_foo(x, uniq) ({ \
	__auto_type _x_##uniq = z; \
	..

#define foo_relay(x, counter) do_foo(x, counter)
#define foo(x) foo_relay(x, __COUNTER__)
is the way to organise it.
Since you don't get a unique number until __COUNTER__ is expanded
inside foo_relay().

	David

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