linux-kernel - Re: [PATCH v4 9/9] bug.h: Convert BUILD_BUG{,_ON} to use BUILD_BUG_ON

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, 31 Oct 2012 00:48:38 -0500
From:	Daniel Santos <danielfsantos@....net>
To:	Josh Triplett <josh@...htriplett.org>
CC:	Borislav Petkov <bp@...en8.de>,
	Daniel Santos <daniel.santos@...ox.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Andi Kleen <ak@...ux.intel.com>,
	Andrea Arcangeli <aarcange@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Christopher Li <sparse@...isli.org>,
	David Daney <david.daney@...ium.com>,
	David Howells <dhowells@...hat.com>,
	Joe Perches <joe@...ches.com>,
	Konstantin Khlebnikov <khlebnikov@...nvz.org>,
	linux-sparse@...r.kernel.org,
	Michel Lespinasse <walken@...gle.com>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Pavel Pisa <pisa@....felk.cvut.cz>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Steven Rostedt <rostedt@...dmis.org>,
	David Rientjes <rientjes@...gle.com>
Subject: Re: [PATCH v4 9/9] bug.h: Convert BUILD_BUG{,_ON} to use BUILD_BUG_ON_MSG

On 10/30/2012 08:02 PM, Josh Triplett wrote:
> On Tue, Oct 30, 2012 at 08:19:05PM +0100, Borislav Petkov wrote:
>> On Sun, Oct 28, 2012 at 03:57:15PM -0500, danielfsantos@....net wrote:
>>> Remove duplicate code by converting BUILD_BUG and BUILD_BUG_ON to just
>>> call BUILD_BUG_ON_MSG.  This not only reduces source code bloat, but
>>> also prevents the possibility of code being changed for one macro and
>>> not for the other (which was previously the case for BUILD_BUG and
>>> BUILD_BUG_ON).
>>>
>>> Signed-off-by: Daniel Santos <daniel.santos@...ox.com>
>>> ---
>>>  include/linux/bug.h |   17 +++--------------
>>>  1 files changed, 3 insertions(+), 14 deletions(-)
>>>
>>> diff --git a/include/linux/bug.h b/include/linux/bug.h
>>> index 3bc1ddf..b58ba51 100644
>>> --- a/include/linux/bug.h
>>> +++ b/include/linux/bug.h
>>> @@ -81,14 +81,8 @@ struct pt_regs;
>>>  #ifndef __OPTIMIZE__
>>>  #define BUILD_BUG_ON(condition) __compiletime_error_fallback(condition)
>>>  #else
>>> -#define BUILD_BUG_ON(condition)						\
>>> -	do {								\
>>> -		extern void __build_bug_on_failed(void)			\
>>> -			__compiletime_error("BUILD_BUG_ON failed");	\
>>> -		__compiletime_error_fallback(condition);		\
>>> -		if (condition)						\
>>> -			__build_bug_on_failed();			\
>>> -	} while(0)
>>> +#define BUILD_BUG_ON(condition) \
>>> +	BUILD_BUG_ON_MSG(condition, "BUILD_BUG_ON failed: " #condition)
>> Concatenating "condition" might not be very informative in all cases.
>> For example:
>>
>> BUILD_BUG_ON(1);
>>
>> Having __LINE__ is good enough IMHO.

Honestly, __LINE__ is only used to keep the function name unique.  If
anything, I think that having it creates more confusion rather than adds
clarity since the error message will indicate the file and line number
anyway.  So in other words, it is redundant without it being apparent
why.  Of course, it's a very simple and portable mechanism to keep the
symbols unique.  IMO, using __COUNTER__would be better for clarity
(since the number wouldn't relate to the anything real), but it is not
portable across versions of gcc (introduced in 4.4 or some such), so we
are using __LINE__.

> While it doesn't always help, it may help sometimes.  Worst case,
> BUILD_BUG_ON(1) gives you no less information than it did before; best
> case, it gives you useful data.

Yeah, and depending upon what it's fed (and how much pre-processing had
been done on that) it can actually prove helpful because stringifying
condition can reveal pre-processing errors as well.  So if you passed
some macro as the condition and it didn't expand the way you expected,
this error message will print out exactly how it expanded, up to the
point of having been passed to BUILD_BUG_ON.  I don't know how much that
could potentially help, but for troubleshooting, I find extra
information helpful, more often than harmful.

Daniel
--
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/