Re: [PATCH v3 2/2] tools/xg: Clean up xend-style overrides for CPU policies

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On 23/05/2024 11:47, Roger Pau Monné wrote:
>> -static xen_cpuid_leaf_t *find_leaf(
>> -    xen_cpuid_leaf_t *leaves, unsigned int nr_leaves,
>> -    const struct xc_xend_cpuid *xend)
>> +static xen_cpuid_leaf_t *find_leaf(xc_cpu_policy_t *p,
>> +                                   const struct xc_xend_cpuid *xend)
>>  {
>>      const xen_cpuid_leaf_t key = { xend->leaf, xend->subleaf };
>>  
>> -    return bsearch(&key, leaves, nr_leaves, sizeof(*leaves), 
>> compare_leaves);
>> +    return bsearch(&key, p->leaves, ARRAY_SIZE(p->leaves),
> 
> Don't you need to use p->nr_leaves here, as otherwise we could check
> against possibly uninitialized leaves (or leaves with stale data)?

Indeed. Good catch (same on the MSR side).

>> -        switch ( p->policy.x86_vendor )
>> +        switch ( cur->policy.x86_vendor )
>>          {
>>          case X86_VENDOR_INTEL:
>> -            for ( i = 0; (p->policy.cache.subleaf[i].type &&
>> -                          i < ARRAY_SIZE(p->policy.cache.raw)); ++i )
>> +            for ( i = 0; (cur->policy.cache.subleaf[i].type &&
>> +                                i < ARRAY_SIZE(cur->policy.cache.raw)); ++i 
>> )
> 
> Nit: indentation is weird here.  I would use:
> 
>             for ( i = 0; cur->policy.cache.subleaf[i].type &&
>                          i < ARRAY_SIZE(cur->policy.cache.raw); ++i )
> 
> Thanks, Roger.

Sure. Leftover from removing the size_t in v2.

Cheers,
Alejandro