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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 20 May 2025, Jan Beulich wrote:
> 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.

Fair enough. Andrew, please add a better commit message. With that:

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>