Re: [PATCH v5 11/30] ARM: GICv3 ITS: introduce device mapping

[Julien Grall <julien.grall@arm.com>]

Hi Andre,

On 06/04/17 17:10, Andre Przywara wrote:
> On 06/04/17 16:34, Julien Grall wrote:
>>> +    if ( ret )
>>> +    {
>>> +        do {
>>> +            i--;
>>> +            gicv3_free_host_lpi_block(dev->host_lpi_blocks[i]);
>>> +            if ( i == 0 )
>>> +                break;
>>> +        } while (1);
>>
>> This could be  } while ( i > 0 ) saving 3 lines.
>
> Argh, I rewrote this sucker three times because "i" is unsigned and I
> apparently missed the easiest solution ;-)
> And still there is a bug in there (if the first allocation fails already
> above). Do you mind if I revert "i" back to a signed int? That would
> allow me to do write "while ( i >= 0 )" here or a for loop.

I was about to say that you can have up 2^32 events (it is silly I 
know), but i represent the block. So I would be ok.

May I ask to add comment? So someone (to not say me) does not go over 
the code and switched to unsigned in the future :).

>>> +
>>> +        goto out;
>>> +    }
>>> +
>>> +    return 0;
>>> +
>>> +out_unlock:
>>> +    spin_unlock(&d->arch.vgic.its_devices_lock);
>>> +
>>> +out:
>>> +    if ( dev )
>>> +    {
>>> +        xfree(dev->pend_irqs);
>>> +        xfree(dev->host_lpi_blocks);
>>> +    }
>>> +    xfree(itt_addr);
>>> +    xfree(dev);
>>> +
>>> +    return ret;
>>> +}
>>> +
>>
>> Cheers,
>>
>>

Cheers,

-- 
Julien Grall

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