linux-kernel - Re: [PATCH 2/2] kernel.h: use __COUNTER__ in min and max macros to avoid -Wshadow warnings

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: <xa1tppf0xvej.fsf@mina86.com>
Date:	Sat, 13 Sep 2014 01:37:40 +0200
From:	Michal Nazarewicz <mina86@...a86.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Mark Rustad <mark.d.rustad@...el.com>,
	"Kirsher\, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	"linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>,
	Hagen Paul Pfeifer <hagen@...u.net>,
	Steven Rostedt <rostedt@...dmis.org>
Subject: Re: [PATCH 2/2] kernel.h: use __COUNTER__ in min and max macros to avoid -Wshadow warnings

On Fri, Sep 12 2014, Andrew Morton <akpm@...ux-foundation.org> wrote:
> On Thu, 11 Sep 2014 23:39:36 +0200 Michal Nazarewicz <mina86@...a86.com> wrote:
>
>> Because min and max macros use the same variable names no matter
>> how many times they are called (or how deep the nesting of their
>> calls), each time min or max calls are nested, the same variables
>> are declared.  This is especially noisy after min3 and max3 have
>> been changed to nest min/max calls.
>> 
>> Using __COUNTER__ solves the problem since each variable will get
>> a unique number aadded to it.  The code will still work even if
>> the compiler does not support __COUNTER__, but then the protection
>> from shadow warning won't work.
>> 
>> The same applies to min_t and max_t macros.
>> 
>> ...
>>
>> --- a/include/linux/kernel.h
>> +++ b/include/linux/kernel.h
>> @@ -695,15 +695,27 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
>>  #endif /* CONFIG_TRACING */
>>  
>>  /*
>> + * Preprocessor magic generating unique identifiers to avoid -Wshadow warnings
>> + * used by min, max, min_t and max_t macros.  cnt is __COUNTER__, op is the
>> + * comparison operator; tx (ty) is type of the first (second) argument,
>> + * xx (yy) is name of a temporary variable to hold the first (second) argument,
>> + * and x (y) is the first (second) argument.
>> + */
>> +#define _min_max_var(cnt, base) _mm_ ## cnt ## base
>> +#define _min_max__(op, tx, xx, x, ty, yy, y) ({		\
>> +	tx xx = (x);					\
>> +	ty yy = (y);					\
>> +	(void) (&xx == &yy);				\
>> +	xx op yy ? xx : yy; })
>> +#define _min_max_(cnt, op, tx, x, ty, y)		\
>> +	_min_max__(op, tx, _min_max_var(cnt, a), x, ty, _min_max_var(cnt, b), y)
>> +#define _min_max(...) _min_max_(__COUNTER__, __VA_ARGS__)
>
> The fact that __COUNTER__ is used in compiler-gcc4.h but not in
> compiler-gcc3.h makes me suspicious about its availability?

http://gcc.gnu.org/ml/gcc-patches/2007-05/msg01579.html so looks like it
has 7 years.  But as the commit message says, the code will still work,
even w/o working __COUNTER__.

> I do think that [1/2] made the code significantly worse-looking

Oh?  I actually thought [1/2] makes it nicer by having a single place
where the min/max logic is implemented.

> and this one is getting crazy.  How useful is W=2 anyway?

I actually do agree with that.  I didn't have high hopes about getting
this patch accepted, but wanted to send it out to show that it can be
done, if it's really deemed useful.

> Has anyone found a bug using it?  The number of warnings in default
> builds is already way too high :(

-- 
Best regards,                                         _     _
.o. | Liege of Serenely Enlightened Majesty of      o' \,=./ `o
..o | Computer Science,  Michał “mina86” Nazarewicz    (o o)
ooo +--<mpn@...gle.com>--<xmpp:mina86@...ber.org>--ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/