Re: [Xen-devel] [PATCH V5] x86/altp2m: Hypercall to set altp2m view visibility

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.03.2020 10:43, Alexandru Stefan ISAILA wrote:
> 
> 
> On 03.03.2020 11:30, Jan Beulich wrote:
>> On 26.02.2020 14:18, Alexandru Stefan ISAILA wrote:
>>> @@ -4840,6 +4841,19 @@ static int do_altp2m_op(
>>>           break;
>>>       }
>>>   
>>> +    case HVMOP_altp2m_set_visibility:
>>> +    {
>>> +        uint16_t idx = a.u.set_visibility.altp2m_idx;
>>
>> Why a fixed width type (and even one inefficient to deal with)?
>> (One might even ask - why a local variable in the first place,
>> when it's used ...
>>
>>> +        if ( a.u.set_visibility.pad )
>>> +            rc = -EINVAL;
>>> +        else if ( !altp2m_active(d) )
>>> +            rc = -EOPNOTSUPP;
>>> +        else
>>> +            rc = p2m_set_altp2m_view_visibility(d, idx,
>>> +                                                
>>> a.u.set_visibility.visible);
>>
>> ... just once here.) The function takes "unsigned int" in any
>> event.
> 
> Sure, I can have this idx dropped and use the value in the structure.
> I had that in place to have line size smaller and the code easy to read.

Dropping the variable is secondary - if you prefer you keep it, so
be it. But if you keep it, its type should by in line with
./CODING_STYLE.

Jan

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