Re: [PATCH] xen: Give compile.h header guards

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.05.2025 23:34, Stefano Stabellini wrote:
> On Mon, 19 May 2025, Jan Beulich wrote:
>> On 19.05.2025 15:52, Andrew Cooper wrote:
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>
>> Is this to please Misra in some way?
>>
>>> --- a/xen/include/xen/compile.h.in
>>> +++ b/xen/include/xen/compile.h.in
>>> @@ -1,3 +1,6 @@
>>> +#ifndef XEN_COMPILE_H
>>> +#define XEN_COMPILE_H
>>> +
>>>  #define XEN_COMPILE_DATE   "@@date@@"
>>>  #define XEN_COMPILE_TIME   "@@time@@"
>>>  #define XEN_COMPILE_BY             "@@whoami@@"
>>> --- a/xen/tools/process-banner.sed
>>> +++ b/xen/tools/process-banner.sed
>>> @@ -12,3 +12,8 @@ s_(.*)_"\1\\n"_
>>>  
>>>  # Trailing \ on all but the final line.
>>>  $!s_$_ \\_
>>> +
>>> +# Append closing header guard
>>> +$a\
>>> +\
>>> +#endif /* XEN_COMPILE_H */
>>
>> This split of #ifndef and #endif is ugly. Can't we switch to something
>> more conventional, like
>>
>> #define XEN_BANNER           "@@banner@@"
>>
>> with the first sed invocation then replacing this by the result of
>> a nested sed invocation using process-banner.sed (which of course would
>> need adjusting some)? (Maybe the double quotes would need omitting here,
>> for process-banner.sed to uniformly apply them.)
> 
> While I agree with Jan that this is kind of ugly, it is a unique case
> and I would prefer an ugly but simple solution than a more complex
> solution. This would be different if we were talking about a widespread
> pattern, in which case the price for complexity would be worth it.
> 
> My 2 cents in this case are in favor of the simplest approach. I would
> ack this patch.

Feel free to do so; my comment was not meant as a plain objection, but more
as a suggestion. The one thing I would ask for is a non-empty description,
though.

Jan