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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.01.19 at 20:19, <sstabellini@xxxxxxxxxx> wrote:
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -312,14 +312,18 @@ extern const initcall_t __initcall_start[], 
> __presmp_initcall_end[],
>  void __init do_presmp_initcalls(void)
>  {
>      const initcall_t *call;
> -    for ( call = __initcall_start; call < __presmp_initcall_end; call++ )
> +    for ( call = __initcall_start;
> +               (unsigned long)call < SYMBOL(__presmp_initcall_end);
> +               call++ )

Hard tabs here and ...

>          (*call)();
>  }
>  
>  void __init do_initcalls(void)
>  {
>      const initcall_t *call;
> -    for ( call = __presmp_initcall_end; call < __initcall_end; call++ )
> +    for ( call = __presmp_initcall_end;
> +               (unsigned long)call < SYMBOL(__initcall_end);
> +               call++ )

... here.

> --- a/xen/common/lib.c
> +++ b/xen/common/lib.c
> @@ -497,7 +497,7 @@ extern const ctor_func_t __ctors_start[], __ctors_end[];
>  void __init init_constructors(void)
>  {
>      const ctor_func_t *f;
> -    for ( f = __ctors_start; f < __ctors_end; ++f )
> +    for ( f = __ctors_start; (unsigned long)f < SYMBOL(__ctors_end); ++f )
>          (*f)();

One of the best examples where SYMBOL() retaining the original type
would help.

Also please take the opportunity and add the missing blank line.

> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -68,7 +68,7 @@ DEFINE_PER_CPU(struct scheduler *, scheduler);
>  DEFINE_PER_CPU(cpumask_t, cpumask_scratch);
>  
>  extern const struct scheduler *__start_schedulers_array[], 
> *__end_schedulers_array[];
> -#define NUM_SCHEDULERS (__end_schedulers_array - __start_schedulers_array)
> +#define NUM_SCHEDULERS (SYMBOL(__end_schedulers_array) - 
> SYMBOL(__start_schedulers_array))

Long line.

> --- a/xen/common/spinlock.c
> +++ b/xen/common/spinlock.c
> @@ -474,7 +474,9 @@ static int __init lock_prof_init(void)
>  {
>      struct lock_profile **q;
>  
> -    for ( q = &__lock_profile_start; q < &__lock_profile_end; q++ )
> +    for ( q = &__lock_profile_start;
> +               (unsigned long)q < SYMBOL(&__lock_profile_end);
> +               q++ )

Hard tabs again.

> --- a/xen/common/virtual_region.c
> +++ b/xen/common/virtual_region.c
> @@ -119,7 +119,11 @@ void __init setup_virtual_regions(const struct 
> exception_table_entry *start,
>          const struct bug_frame *s;
>  
>          s = bug_frames[i - 1];
> -        sz = bug_frames[i] - s;
> +        /*
> +         * Cast to unsigned long to calculate the size to avoid
> +         * subtractions between pointers pointing to different objects.
> +         */
> +        sz = (unsigned long)bug_frames[i] - (unsigned long)s;

Perhaps better to use SYMBOL() in the definition of bug_frames[]?

> --- 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) ({                                             \
> +    const unsigned long __p = (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) ({                                        \
> +    const unsigned long __p = (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 unsigned long __p = (unsigned long)(p);                   \
> +    (__p >= SYMBOL(_srodata)) && (__p < SYMBOL(_erodata));      \
>  })
>  
>  extern char _sinittext[], _einittext[];
> -#define is_kernel_inittext(p) ({                \
> -    char *__p = (char *)(unsigned long)(p);     \
> -    (__p >= _sinittext) && (__p < _einittext);  \
> +#define is_kernel_inittext(p) ({                                    \
> +    const unsigned long __p = (unsigned long)(p);                   \
> +    (__p >= SYMBOL(_sinittext)) && (__p < SYMBOL(_einittext));  \
>  })

If you fully replace the macro bodies anyway, please take the
opportunity and also do away with the name space violating
leading underscores (use trailing ones instead, for example).

Jan


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