Re: [Xen-devel] [PATCH v6 4/4] xen/common: use SYMBOL when required

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 10 Jan 2019, Jan Beulich wrote:
> >>> On 10.01.19 at 00:42, <sstabellini@xxxxxxxxxx> wrote:
> > --- a/xen/common/version.c
> > +++ b/xen/common/version.c
> > @@ -147,14 +147,14 @@ static int __init xen_build_init(void)
> >      int rc;
> >  
> >      /* --build-id invoked with wrong parameters. */
> > -    if ( __note_gnu_build_id_end <= &n[0] )
> > +    if ( SYMBOL(__note_gnu_build_id_end) <= &n[0] )
> >          return -ENODATA;
> >  
> >      /* Check for full Note header. */
> > -    if ( &n[1] >= __note_gnu_build_id_end )
> > +    if ( &n[1] >= SYMBOL(__note_gnu_build_id_end) )
> >          return -ENODATA;
> >  
> > -    sz = (void *)__note_gnu_build_id_end - (void *)n;
> > +    sz = (void *)SYMBOL(__note_gnu_build_id_end) - (void *)n;
> 
> Now this is an instance where I wouldn't mind if you switched the
> casts to (unsigned long).

OK


> > --- a/xen/common/virtual_region.c
> > +++ b/xen/common/virtual_region.c
> > @@ -103,13 +103,13 @@ void __init setup_virtual_regions(const struct 
> > exception_table_entry *start,
> >  {
> >      size_t sz;
> >      unsigned int i;
> > -    static const struct bug_frame *const __initconstrel bug_frames[] = {
> > -        __start_bug_frames,
> > -        __stop_bug_frames_0,
> > -        __stop_bug_frames_1,
> > -        __stop_bug_frames_2,
> > +    const struct bug_frame *bug_frames[] = {
> 
> Please don't loose the second const.

OK


> > --- a/xen/include/xen/kernel.h
> > +++ b/xen/include/xen/kernel.h
> > @@ -66,27 +66,27 @@
> >  })
> >  
> >  extern char _start[], _end[], start[];
> > -#define is_kernel(p) ({                         \
> > -    char *__p = (char *)(unsigned long)(p);     \
> > -    (__p >= _start) && (__p < _end);            \
> > +#define is_kernel(p) ({                                             \
> > +    char *p__ = (char *)(unsigned long)(p);                         \
> > +    (p__ >= SYMBOL(_start)) && (p__ < SYMBOL(_end));                \
> >  })
> >  
> >  extern char _stext[], _etext[];
> > -#define is_kernel_text(p) ({                    \
> > -    char *__p = (char *)(unsigned long)(p);     \
> > -    (__p >= _stext) && (__p < _etext);          \
> > +#define is_kernel_text(p) ({                                        \
> > +    char *p__ = (char *)(unsigned long)(p);                         \
> > +    (p__ >= SYMBOL(_stext)) && (p__ < SYMBOL(_etext));              \
> >  })
> >  
> >  extern const char _srodata[], _erodata[];
> > -#define is_kernel_rodata(p) ({                  \
> > -    const char *__p = (const char *)(unsigned long)(p);     \
> > -    (__p >= _srodata) && (__p < _erodata);      \
> > +#define is_kernel_rodata(p) ({                                      \
> > +    const char *p__ = (const char *)(unsigned long)(p);             \
> 
> Just like here, in all other of the sibling macros you could easily
> have switched p__ to be const char * as well.

OK


> With at least the bug_frames[] remark taken care of
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
 
I'll make the changes above and add your reviewed-by

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