Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 10 Jan 2019, Jan Beulich wrote:
> >>> On 10.01.19 at 00:42, <sstabellini@xxxxxxxxxx> wrote:
> > --- a/xen/include/xen/compiler.h
> > +++ b/xen/include/xen/compiler.h
> > @@ -99,6 +99,16 @@
> >      __asm__ ("" : "=r"(__ptr) : "0"(ptr));      \
> >      (typeof(ptr)) (__ptr + (off)); })
> >  
> > +/*
> > + * Similar to RELOC_HIDE, but written to be used with symbols such as
> > + * _stext and _etext to avoid undefined behavior comparing pointers to
> > + * different objects. It can handle array types.
> > + */
> > +#define SYMBOL(ptr)                               \
> > +  ({ unsigned long __ptr;                       \
> > +    __asm__ ("" : "=r"(__ptr) : "0"(ptr));      \
> > +    (typeof(*(ptr)) *) (__ptr); })
> 
> I'm sorry for thinking of this only now, but SYMBOL() is (no longer)
> appropriate as a name here. I'd like to suggest SYMBOL_HIDE(), in
> line with the other macro's name. With that and with
> - the stray blank after the cast dropped
> - the asm() formatting corrected; there are a number of blanks
>   missing
> - the name of the local variable corrected as per my original
>   suggestion
> - indentation corrected
> - the use of underscores on "asm" and "typeof" brought in line
> you may (re-)add
> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> Furthermore I wonder why the cast is needed in the first place.
> Doesn't
> 
> #define SYMBOL_HIDE(ptr) ({                   \
>     __typeof__(*(ptr)) *ptr_;                 \
>     __asm__ ( "" : "=r" (ptr_) : "0" (ptr) ); \
>     ptr_; \
> })
> 
> do the job as well?

It works, but it goes even more in the direction of not fixing MISRA-C
compliance. Given that we have already enstblished that we'll have to
check with the experts on this topic, I am OK with using this
implementation. Changing the implementation of SYMBOL_HIDE in the future
is easy if we need to. I added your Acked-by.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel