Re: [PATCH 14/21] libs/guest: introduce helper to check cpu policy compatibility

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.03.2021 14:40, Roger Pau Monné wrote:
> On Tue, Mar 30, 2021 at 06:02:45PM +0200, Jan Beulich wrote:
>> On 23.03.2021 10:58, Roger Pau Monne wrote:
>>> --- a/tools/libs/guest/xg_cpuid_x86.c
>>> +++ b/tools/libs/guest/xg_cpuid_x86.c
>>> @@ -1098,3 +1098,20 @@ int xc_cpu_policy_update_msrs(xc_interface *xch, 
>>> xc_cpu_policy_t policy,
>>>      return rc;
>>>  
>>>  }
>>> +
>>> +bool xc_cpu_policy_is_compatible(xc_interface *xch, const xc_cpu_policy_t 
>>> p1,
>>> +                                 const xc_cpu_policy_t p2)
>>> +{
>>> +    struct cpu_policy_errors err;
>>
>> Don't you need an initializer here for ...
>>
>>> +    int rc = x86_cpu_policies_are_compatible(p1, p2, &err);
>>> +
>>> +    if ( !rc )
>>> +        return true;
>>> +
>>> +    if ( err.leaf != -1 )
>>> +        ERROR("Leaf %#x subleaf %#x is not compatible", err.leaf, 
>>> err.subleaf);
>>> +    if ( err.msr != -1 )
>>> +        ERROR("MSR index %#x is not compatible", err.msr);
>>
>> ... these checks to have a chance of actually triggering? (I'm also
>> not certain -1 is a good indicator, but I guess we have been using it
>> elsewhere as well.)
> 
> Well, this is strictly the error path, at which point I would expect
> err to be properly set by x86_cpu_policies_are_compatible. I can
> however initialize err for safety here.

Aiui in the error case one of the two, but not both fields would
be set. Without initializer you'd likely find both of them != -1,
though.

Jan