linux-kernel - Re: [PATCH v5 8/9] compiler.h, bug.h: Prevent double error messages with BUILD_BUG{,

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: <20121117143918.GC16441@x1.osrc.amd.com>
Date:	Sat, 17 Nov 2012 15:39:18 +0100
From:	Borislav Petkov <bp@...en8.de>
To:	Daniel Santos <daniel.santos@...ox.com>
Cc:	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>,
	Josh Triplett <josh@...htriplett.org>,
	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>
Subject: Re: [PATCH v5 8/9] compiler.h, bug.h: Prevent double error messages
 with BUILD_BUG{,_ON}

On Thu, Nov 15, 2012 at 01:44:45PM -0600, Daniel Santos wrote:
> Yeah, I agree. Also, with the complexity, I think a few more comments
> can be helpful in compiler.h to clarify what these macros are for more
> specifically.

>From what I've read so far the comments are fine but if you want to be
more descriptive then sure, by all means. Just don't let them grow out
of proportion - keep them simple and succinct.

> On another note, I have a "part two" set of patches for bug.h &
> compiler*.h that does some other stuff (more cleanup & restructuring)
> and this is making me think about that more.  My thought about
> __compiletime_error_fallback is that it should be called only from
> within compiler.h (as in the following patch) and basically just be a
> private macro.  However, we still use the use the negative sized array
> trick for the unoptimized version of BUILD_BUG_ON (which may have
> limited meaning), and we also use a negative bit specifier on a bitfield
> in BUILD_BUG_ON_ZERO and BUILD_BUG_ON_NULL (which I treat some in my
> other patches as well).  But my thought is that it may be helpful to
> encapsulate these tricks into (public) macros in compiler*.h, such as
> "compiletime_assert_negarray" and "compiletime_assert_negbitfiled" and
> then have __compiletime_error_fallback expand to
> compiletime_assert_negarray when it's needed and no-op when it's not.
> 
> This doesn't have to be decided now, but it's just a thought you gave me.

I dunno. I'm thinking that maybe making them more unreadable for no
apparent reason might be simply too much. They're fine the way they are
now, if you ask me.

> And in case you're wondering about the negative bit field,
> BUILD_BUG_ON_{ZERO,NULL} can't call BUILD_BUG_ON in a complex expression
> ({ exp; exp; }) because it is evaluated outside of a function body and
> gcc doesn't like that.  Thus, the following macro would, sadly, result
> in errors:
> 
> #define BUILD_BUG_ON_ZERO(exp) ({BUILD_BUG_ON(exp); 0;})
> 
> However, it would not be an error to call an __alwaysinline function
> that uses BUILD_BUG_ON, so I still have to explore that and make sure it
> works in all scenarios (and compilers).

Ok, I see your point. Think of it this way: if unifying those macros can
only happen at the expense of readability or performance then maybe it
is not worth the trouble. If, OTOH, you can think of something slick and
pretty, then go ahead :).

HTH.

-- 
Regards/Gruss,
Boris.
--
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/