7:02 a.m.
This patch series clarifies the virBitmapLastSetBit function and adds some
tests for the empty bitmap.
It also clarifies the documentations of virBitmapNewQuiet and virBitmapNew.
Marc Hartmayer (3):
util: bitmap: clarify virBitmapLastSetBit() behavior for empty bitmaps
util: bitmap: Mention the size == 0 handling
tests: Add test cases for the empty bitmap
src/util/virbitmap.c | 18 +++++++++++-------
tests/virbitmaptest.c | 17 +++++++++++++++++
2 files changed, 28 insertions(+), 7 deletions(-)
--
2.5.5
7:02 a.m.
New subject: [libvirt] [PATCH 1/3] 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 <fiuczy(a)linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
---
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 3276763..2029a73 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -952,7 +952,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;
@@ -971,8 +975,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--) {
--
2.5.5
7:02 a.m.
New subject: [libvirt] [PATCH 2/3] util: bitmap: Mention the size == 0 handling
As there is an explicit constructor for the special case of empty
bitmaps, we should mention that the generic constructors rejects the
creation of empty bitmaps.
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe(a)linux.vnet.ibm.com>
Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
---
src/util/virbitmap.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 2029a73..3b85c16 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -60,8 +60,8 @@ struct _virBitmap {
*
* Allocate a bitmap capable of containing @size bits.
*
- * Returns a pointer to the allocated bitmap or NULL if memory cannot be
- * allocated. Does not report libvirt errors.
+ * Returns a pointer to the allocated bitmap or NULL if either memory cannot be
+ * allocated or size is 0. Does not report libvirt errors.
*/
virBitmapPtr
virBitmapNewQuiet(size_t size)
@@ -95,8 +95,8 @@ virBitmapNewQuiet(size_t size)
*
* Allocate a bitmap capable of containing @size bits.
*
- * Returns a pointer to the allocated bitmap or NULL if memory cannot be
- * allocated. Reports libvirt errors.
+ * Returns a pointer to the allocated bitmap or NULL if either memory cannot be
+ * allocated or size is 0. Reports libvirt errors.
*/
virBitmapPtr
virBitmapNew(size_t size)
--
2.5.5
7:02 a.m.
New subject: [libvirt] [PATCH 3/3] tests: Add test cases for the empty bitmap
As the empty bitmap exists, we should also test it. This patch adds
test cases for the procedures 'virBitmapNextSetBit',
'virBitmapLastSetBit',
'virBitmapNextClearBit'.
Tested-by: Sascha Silbe <silbe(a)linux.vnet.ibm.com>
Reviewed-by: Sascha Silbe <silbe(a)linux.vnet.ibm.com>
Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
---
tests/virbitmaptest.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
index 009fa0d..a17ef82 100644
--- a/tests/virbitmaptest.c
+++ b/tests/virbitmaptest.c
@@ -191,6 +191,23 @@ test4(const void *data ATTRIBUTE_UNUSED)
if (ARRAY_CARDINALITY(bitsPos) + ARRAY_CARDINALITY(bitsPosInv) != size)
goto error;
+ /* 0. empty set */
+
+ if (!(bitmap = virBitmapNewEmpty()))
+ goto error;
+
+ if (virBitmapNextSetBit(bitmap, -1) != -1)
+ goto error;
+
+ if (virBitmapLastSetBit(bitmap) != -1)
+ goto error;
+
+ if (virBitmapNextClearBit(bitmap, -1) != -1)
+ goto error;
+
+ virBitmapFree(bitmap);
+ bitmap = NULL;
+
/* 1. zero set */
bitmap = virBitmapNew(size);
--
2.5.5
4:09 a.m.
On 06.07.2016 14:02, Marc Hartmayer wrote:
This patch series clarifies the virBitmapLastSetBit function and adds
some
tests for the empty bitmap.
It also clarifies the documentations of virBitmapNewQuiet and virBitmapNew.
Marc Hartmayer (3):
util: bitmap: clarify virBitmapLastSetBit() behavior for empty bitmaps
util: bitmap: Mention the size == 0 handling
tests: Add test cases for the empty bitmap
src/util/virbitmap.c | 18 +++++++++++-------
tests/virbitmaptest.c | 17 +++++++++++++++++
2 files changed, 28 insertions(+), 7 deletions(-)
ACKed all three and pushed.
Michal
3089
days inactive
3092
days old
4 comments
2 participants
participants (2)
-
Marc Hartmayer -
Michal Privoznik