From 598845b4f6ae69e237a4f5d65b22e3df2ba5c2e9 Mon Sep 17 00:00:00 2001 From: Marc Hartmayer Date: Wed, 6 Jul 2016 14:02:26 +0200 Subject: util: bitmap: clarify virBitmapLastSetBit() behavior for empty bitmaps Before the variable 'bits' was initialized with 0 (commit 3470cd860d517760b13e26d97b6a842ff72687a1), the following bug was possible. A function call with an empty bitmap leads to undefined behavior. Because if 'bitmap->map_len == 0' 'unusedBits' will be <= 0 and 'sz == 1'. So the non global and non static variable 'bits' would have never been set. Consequently the check 'bits == 0' results in undefined behavior. This patch clarifies the current version of the function by handling the empty bitmap explicitly. Also, for an empty bitmap there is obviously no bit set so we can just return -1 (indicating no bit set) right away. The explicit check for 'bits == 0' after the loop is unnecessary because we only get to this point if no set bit was found. Reviewed-by: Boris Fiuczynski Reviewed-by: Sascha Silbe Reviewed-by: Bjoern Walk Signed-off-by: Marc Hartmayer (cherry picked from commit 7cd01a248b3995909adad29a6edbe76d3d16510f) --- src/util/virbitmap.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index c94764792f..3610feb4d0 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -724,7 +724,11 @@ virBitmapLastSetBit(virBitmapPtr bitmap) ssize_t i; int unusedBits; ssize_t sz; - unsigned long bits = 0; + unsigned long bits; + + /* If bitmap is empty then there is no set bit */ + if (bitmap->map_len == 0) + return -1; unusedBits = bitmap->map_len * VIR_BITMAP_BITS_PER_UNIT - bitmap->max_bit; @@ -743,8 +747,8 @@ virBitmapLastSetBit(virBitmapPtr bitmap) goto found; } - if (bits == 0) - return -1; + /* Only reached if no set bit was found */ + return -1; found: for (i = VIR_BITMAP_BITS_PER_UNIT - 1; i >= 0; i--) { -- cgit v1.2.1