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

The original implementation is extremely straightforward but not too efficient, because it uses the public API instead of poking the innards directly. This second implementation does the latter, and as a consequence can afford to make @b const, which is nice even though most existing virBitmap APIs use non-const pointers even for read-only bitmaps.

That said, I'm not entirely sure I got it quite right, so a review from someone who's well versed in the virBitmap internals (*cough* Peter *cough*) would be appreciated, and if my implementation passes muster or can be amended through review comments I'll gladly squash this commit into the previous one.

Signed-off-by: Andrea Bolognani <abologna(a)redhat.com&gt; --- src/util/virbitmap.c | 18 ++++++++++++------ src/util/virbitmap.h | 2 +- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index 1f0db563ab..f2127c053d 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -1271,17 +1271,23 @@ virBitmapIntersect(virBitmapPtr a, */ int virBitmapUnion(virBitmapPtr a, - virBitmapPtr b) + const virBitmap *b) { size_t i; + size_t max; - for (i = 0; i < b->nbits; i++) { - if (virBitmapIsBitSet(b, i)) { - if (virBitmapSetBitExpand(a, i) < 0) - return -1; - } + if (a->nbits < b->nbits && + virBitmapExpand(a, b->nbits) < 0) {

+ return -1; } + max = a->map_len; + if (max > b->map_len) + max = b->map_len;

+ + for (i = 0; i < max; i++) + a->map[i] |= b->map[i]; + return 0; }