Re: [Xen-devel] [PATCH v11 5/9] xen/x86: use DECLARE_BOUNDS as required

[Ian Jackson <ian.jackson@xxxxxxxxxx>]

Stefano Stabellini writes ("[PATCH v11 5/9] xen/x86: use DECLARE_BOUNDS as 
required"):
> Use DECLARE_BOUNDS and the two static inline functions that come with it
> for comparisons and subtractions of:
> 
> __2M_rwdata_start, __2M_rwdata_end, __end_vpci_array,
> __start_vpci_array, _stextentry, _etextentry, __trampoline_rel_start,
> __trampoline_rel_stop, __trampoline_seg_start, __trampoline_seg_stop
> __per_cpu_start, __per_cpu_data_end
> 
> possible to use the provided static inline functions.
> M3CM: Rule-18.2: Subtraction between pointers shall only be applied to
> pointers that address elements of the same array.

Hi.  I picked one of these to briefly look at the detail of and I
spotted this:

> @@ -49,7 +50,7 @@ static void _free_percpu_area(struct rcu_head *head)
>  {
>      struct free_info *info = container_of(head, struct free_info, rcu);
>      unsigned int cpu = info->cpu;
> -    char *p = __per_cpu_start + __per_cpu_offset[cpu];
> +    char *p = (char *)__per_cpu_start + __per_cpu_offset[cpu];

This is dangerous because it turns __per_cpu_start which is a magic
linker symbol pointer into one which you might try to compare with
other pointers.

I can't see from your patch what else is done to `p'.  Something ought
to be done to prevent future programmers from comparing p to other
pointers.

Some extended comment, or something, is likely to be needed every time
you introduce a cast.

>      free_xenheap_pages(p, PERCPU_ORDER);

JOOI, why does free_xenheap_pages not take a void* ?

Thanks,
Ian.

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