Re: [PATCH v5 8/9] compiler.h, bug.h: Prevent double error messageswith BUILD_BUG{,_ON}

[Borislav Petkov]

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/