[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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.



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.