On 26/06/2024 10:23 am, Jan Beulich wrote:
> On 25.06.2024 21:07, Andrew Cooper wrote:
>> --- a/xen/include/xen/macros.h
>> +++ b/xen/include/xen/macros.h
>> @@ -59,6 +59,8 @@
>> #define BUILD_BUG_ON(cond) ((void)BUILD_BUG_ON_ZERO(cond))
>> #endif
>>
>> +#define BUILD_ERROR(msg) asm ( ".error \"" msg "\"" )
> I think this wants a comment, and one even beyond what is said for
> BUILD_BUG_ON(). This is primarily to make clear to people when to use
> which, i.e. I consider it important to mention here that it is intended
> for code which, in the normal case, we expect to be DCE-d. The nature
> of the condition used is also a relevant factor, as in some cases
> BUILD_BUG_ON() simply cannot be used because something that really is
> compile time constant isn't an integer constant expression. With
> something to this effect:
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
Done, thanks.
>
> I have another question / suggestion, though.
>
>> --- a/xen/include/xen/self-tests.h
>> +++ b/xen/include/xen/self-tests.h
>> @@ -22,9 +22,9 @@
>> typeof(fn(val)) real = fn(val); \
>> \
>> if ( !__builtin_constant_p(real) ) \
>> - asm ( ".error \"'" STR(fn(val)) "' not compile-time constant\""
>> ); \
>> + BUILD_ERROR("'" STR(fn(val)) "' not compile-time constant"); \
>> else if ( real != res ) \
>> - asm ( ".error \"Compile time check '" STR(fn(val) == res) "'
>> failed\"" ); \
>> + BUILD_ERROR("Compile time check '" STR(fn(val) == res) "'
>> failed"); \
>> } while ( 0 )
> While right here leaving the condition outside of the macro is
> perhaps more natural, considering a case where there's just an if()
> I wonder whether we shouldn't also (only?) have BUILD_ERROR_ON(),
> better paralleling BUILD_BUG_ON():
Right now none of the uses in this series, nor any of the MISRA
conversions away __bad_*() want an _ON() form.
Nothing stops us adding an _ON() form in the future if a reasonable
usecase appears, but right now there's no need.
~Andrew
