Re: [XEN PATCH v15 2/4] x86/irq: allow setting IRQ permissions from GSI instead of pIRQ

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.09.2024 12:34, Daniel P. Smith wrote:
> On 9/11/24 02:58, Jiqian Chen wrote:
>> @@ -237,6 +238,34 @@ long arch_do_domctl(
>>           break;
>>       }
>>   
>> +    case XEN_DOMCTL_gsi_permission:
>> +    {
>> +        int irq;
>> +        unsigned int gsi = domctl->u.gsi_permission.gsi;
>> +        uint32_t flags = domctl->u.gsi_permission.flags;
>> +
>> +        /* Check only valid bits are set */
>> +        ret = -EINVAL;
>> +        if ( flags & ~XEN_DOMCTL_GSI_ACTION_MASK )
>> +            break;
>> +
>> +        ret = irq = gsi_2_irq(gsi);
> 
> I was recently informed that a = b = c; form is not MISRA compliant. 
> Since you just overwrite ret after the check, why not drop the 
> assignment to ret and mae the next check against irq instead.

The Misra concern is valid, yet the suggestion doesn't look to be quite
matching what is needed. After all if we take ...

>> +        if ( ret <= 0 )
>> +            break;

... the "break" path, "ret" needs to be set. Plus there's the problem of
"ret" being zero when exiting the function indicates success, yet this
is an error path (requiring ret < 0). So overall perhaps

         irq = gsi_2_irq(gsi);
         if ( irq <= 0 )
         {
             ret = irq ?: -EACCES;
             break;
         }

?

Jan