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

[Julien Grall <julien.grall@xxxxxxx>]

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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel