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

[Rustad, Mark D]

Michal,

On Sep 12, 2014, at 4:37 PM, Michal Nazarewicz <mina86@xxxxxxxxxx> wrote:

> On Fri, Sep 12 2014, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>> On Thu, 11 Sep 2014 23:39:36 +0200 Michal Nazarewicz <mina86@xxxxxxxxxx> 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.

It does have that going for it.

>> 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.

Well, I learned something from it. Thank you for teaching this old dog a new trick.

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

-- 
Mark Rustad, Networking Division, Intel Corporation

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail