Re: [Xen-devel] [PATCH] domctl: fix IRQ permission granting/revocation

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 10.12.14 at 10:53, <Ian.Campbell@xxxxxxxxxxxxx> wrote:
> On Wed, 2014-12-10 at 08:07 +0000, Jan Beulich wrote:
>> --- a/xen/common/domctl.c
>> +++ b/xen/common/domctl.c
>> @@ -981,18 +981,18 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>>  
>>      case XEN_DOMCTL_irq_permission:
>>      {
>> -        unsigned int pirq = op->u.irq_permission.pirq;
>> +        unsigned int pirq = op->u.irq_permission.pirq, irq;
>>          int allow = op->u.irq_permission.allow_access;
>>  
>>          if ( pirq >= d->nr_pirqs )
>>              ret = -EINVAL;
>> -        else if ( !pirq_access_permitted(current->domain, pirq) ||
>> +        else if ( !(irq = pirq_access_permitted(current->domain, pirq)) ||
> 
> I find hiding an assignment inside the second condition in a chain of
> if's to be rather obfuscated. Doing an assignment in a standalone if
> statement is one thing, this is going to far IMHO.

Changed. My main intention was to avoid having to add another
"break;".

> Also, you range check pirq against d->nr_pirqs but then translate it
> against current->domain, is that correct?

No, it's not. As much as xsm_irq_permission(XSM_HOOK, d, pirq, allow)
is bogus, yet it's not clear to me whether switching that to use the Xen
IRQ number is okay without any other changes. Daniel?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel