Re: [Xen-devel] [PATCH v8 4/7] xen: use initcall_start_, ctors_start_, and more

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 16.01.19 at 00:35, <sstabellini@xxxxxxxxxx> wrote:
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -306,20 +306,23 @@ void add_taint(unsigned int flag)
>      tainted |= flag;
>  }
>  
> -extern const initcall_t __initcall_start[], __presmp_initcall_end[],
> -    __initcall_end[];
> +extern uintptr_t initcall_start_, presmp_initcall_end_, initcall_end_;
>  
>  void __init do_presmp_initcalls(void)
>  {
>      const initcall_t *call;
> -    for ( call = __initcall_start; call < __presmp_initcall_end; call++ )
> +    for ( call = (const initcall_t *)initcall_start_;
> +          (uintptr_t)call < presmp_initcall_end_;

This (just taken as an example) is at best marginally better
than using casts to an arithmetic type without intermediate
variables.

> --- a/xen/common/schedule.c
> +++ b/xen/common/schedule.c
> @@ -67,9 +67,10 @@ DEFINE_PER_CPU(struct scheduler *, scheduler);
>  /* Scratch space for cpumasks. */
>  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 schedulers __start_schedulers_array
> +extern uintptr_t start_schedulers_array_, end_schedulers_array_;
> +#define NUM_SCHEDULERS ((end_schedulers_array_ - start_schedulers_array_) \
> +                        / sizeof(const struct scheduler *))

Despite not being a cast, this is another example of a hidden
type dependency which would better not be introduced. At
the very least this should use an expression rather than a
type name, such that the type of the expression changing
also affects the calculation here. Granted it's a pointer here,
so e.g. renaming struct scheduler wouldn't have bad
consequences, but other (future) places may clone this code
and use other than a pointer.

As another general remark - I'm also not really happy about
the trailing underscores. Yes, this is a means to avoid leading
ones, and hence to avoid name space violations. But we use
trailing underscores in macros a lot, so this opens up new
conflict potential. Anyway, I still hope we can get away either
without any intermediate variables, or ones replacing (rather
than amending) the current ones.

Jan



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