Re: [PATCH v4 9/9] bug.h: Convert BUILD_BUG{,_ON} to use BUILD_BUG_ON_MSG

[Daniel Santos <danielfsantos@att.net>]

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@att.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@pobox.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