[libvirt] [PATCH 2/4] fixup? util: Optimize virBitmapUnion()

[Andrea Bolognani]

On Mon, 2019-06-03 at 14:10 +0200, Ján Tomko wrote:
> On Fri, May 31, 2019 at 05:22:00PM +0200, Andrea Bolognani wrote:
> > +    if (a->nbits < b->nbits &&
> > +        virBitmapExpand(a, b->nbits) < 0) {
> 
> After this, 'b' can hold b->nbits and 'a' can hold b->nbits+1.
> 
> if (b->nbits &&
>     a->nbits < b->nbits &&
>     virBitmapExpand(a, b->nbits -1) < 0) {

Yeah, you're right, we need to account for the zero-indexing of bits.
I mean, it's not like the resulting bitmap would be incorrect either
way, but we might end up allocating more memory than it's actually
required.

The first check seems unnecessary, though: the only case in which the
argument to virBitmapExpand() would be incorrect is b->nbits == 0,
but we know that both a->nbits and b->nbits are >= 0 and we also just
verified that a->nbits < b->nbits, so b->nbits must be >= 1 and the
argument to virBitmapExpand() will always be correct. Or am I missing
something?

> > +    max = a->map_len;
> > +    if (max > b->map_len)
> > +        max = b->map_len;
> 
> You expanded the 'a' bitmap just a few lines above.
> So if you can safely go up to b->map_len without accessing
> invalid memory in a or missing any set bits in b.

I thought the above would be necessary to cover the case where a
is bigger than b, but I realize now that iterating over the smaller
b will deal with such a situation just fine. So nice catch, and
consider it fixed :)

-- 
Andrea Bolognani / Red Hat / Virtualization