On Thu, 10 Jan 2019, Jan Beulich wrote: > >>> On 10.01.19 at 00:42, <sstabell...@kernel.org> 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 <jbeul...@suse.com> > > 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@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel