Re: [Xen-devel] [PATCH 3/6] add check to cpumask_of()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.01.18 at 18:43, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 19/01/18 16:04, Jan Beulich wrote:
>> Just like any other function's CPU inputs, the one here shouldn't go
>> unchecked.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>
>> --- a/xen/include/xen/cpumask.h
>> +++ b/xen/include/xen/cpumask.h
>> @@ -304,7 +304,9 @@ extern const unsigned long
>>  
>>  static inline const cpumask_t *cpumask_of(unsigned int cpu)
>>  {
>> -    const unsigned long *p = cpu_bit_bitmap[1 + cpu % BITS_PER_LONG];
>> +    const unsigned long *p = cpu_bit_bitmap[1 + cpumask_check(cpu) %
>> +                                                    BITS_PER_LONG];
>> +
>>      return (const cpumask_t *)(p - cpu / BITS_PER_LONG);
>>  }
> 
> This would be slightly easier to read as
> 
> diff --git a/xen/include/xen/cpumask.h b/xen/include/xen/cpumask.h
> index 3f340d6..7507ae9 100644
> --- a/xen/include/xen/cpumask.h
> +++ b/xen/include/xen/cpumask.h
> @@ -304,7 +304,9 @@ extern const unsigned long
>  
>  static inline const cpumask_t *cpumask_of(unsigned int cpu)
>  {
> -       const unsigned long *p = cpu_bit_bitmap[1 + cpu % BITS_PER_LONG];
> +       const unsigned long *p =
> +            cpu_bit_bitmap[1 + cpumask_check(cpu) % BITS_PER_LONG];
> +
>         return (const cpumask_t *)(p - cpu / BITS_PER_LONG);
>  }

I'm not convinced; looking at the patch again I did realize though
that indentation of the 2nd line was off by one.

> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

I'd prefer to keep it the way it is (with indentation fixed). Please
let me know whether you outright object, or whether I'm fine to
add your R-b, or whether I'm fine to commit it with Wei's alone.

Jan


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