[refactor/test] speed up and improve bitmap tests

The bitmap tests were slowing down the entire pipeline, while at the
same time missing important checks on the inputs. This version is a bit
more exhaustive, while also speeding up the test by only checking the
corner-cases (first bits, last bits, partial ranges, overlapping
ranges).

include/aml/utils/bitmap.h

@@ -27,9 +27,9 @@
 /** The type used to store bits **/
 #define AML_BITMAP_TYPE  unsigned long
 /** The number of basic type elements used to store bits **/
-#define AML_BITMAP_SIZE  (AML_BITMAP_BYTES/sizeof(AML_BITMAP_TYPE))
+#define AML_BITMAP_SIZE  ((int)(AML_BITMAP_BYTES/sizeof(AML_BITMAP_TYPE)))
 /** The number of bits held in each basic type element **/
-#define AML_BITMAP_NBITS (8 * sizeof(AML_BITMAP_TYPE))
+#define AML_BITMAP_NBITS ((int)(8 * sizeof(AML_BITMAP_TYPE)))
 
 /**
  * aml_bitmap is a static array of elements wrapped in a structure.
@@ -50,13 +50,13 @@ void aml_bitmap_copy(struct aml_bitmap *dst, const struct aml_bitmap *src);
  * Empty a bitmap with all bits cleared.
  * @param bitmap: The bitmap to set.
  **/
-void aml_bitmap_zero(struct aml_bitmap *bitmap);
+int aml_bitmap_zero(struct aml_bitmap *bitmap);
 
 /**
  * Fill a bitmap with all bits set.
  * @param bitmap: The bitmap to set.
  **/
-void aml_bitmap_fill(struct aml_bitmap *bitmap);
+int aml_bitmap_fill(struct aml_bitmap *bitmap);
 
 /**
  * Check whether a bit in bitmap is set.
@@ -135,7 +135,7 @@ int aml_bitmap_clear_range(struct aml_bitmap *bitmap,
  * @param bitmap: The bitmap to inspect.
  * @return The number of bits set in bitmap.
  **/
-unsigned long aml_bitmap_nset(const struct aml_bitmap *bitmap);
+int aml_bitmap_nset(const struct aml_bitmap *bitmap);
 
 /**
  * Copy a unsigned long array used as a bitmap into an actual bitmap.

src/utils/bitmap.c

@@ -50,13 +50,18 @@ void aml_bitmap_copy_to_ulong(const struct aml_bitmap *dst,
 			src[AML_BITMAP_NTH(i)] |= (1UL << AML_BITMAP_ITH(i));
 }
 
-void aml_bitmap_zero(struct aml_bitmap *bitmap)
+int aml_bitmap_zero(struct aml_bitmap *bitmap)
 {
+	if (bitmap == NULL)
+		return -AML_EINVAL;
 	memset(bitmap, 0, sizeof(struct aml_bitmap));
+	return 0;
 }
 
 int aml_bitmap_iszero(const struct aml_bitmap *bitmap)
 {
+	if (bitmap == NULL)
+		return -AML_EINVAL;
 	for (unsigned int i = 0; i < AML_BITMAP_SIZE; i++)
 		if (bitmap->mask[i] != AML_BITMAP_EMPTY)
 			return 0;
@@ -65,35 +70,46 @@ int aml_bitmap_iszero(const struct aml_bitmap *bitmap)
 
 int aml_bitmap_isfull(const struct aml_bitmap *bitmap)
 {
+	if (bitmap == NULL)
+		return -AML_EINVAL;
 	for (unsigned int i = 0; i < AML_BITMAP_SIZE; i++)
 		if (bitmap->mask[i] != AML_BITMAP_FULL)
 			return 0;
 	return 1;
 }
 
-void aml_bitmap_fill(struct aml_bitmap *bitmap)
+int aml_bitmap_fill(struct aml_bitmap *bitmap)
 {
+	if (bitmap == NULL)
+		return -AML_EINVAL;
 	memset(bitmap, ~0, sizeof(struct aml_bitmap));
+	return 0;
 }
 
 int aml_bitmap_isset(const struct aml_bitmap *bitmap, const unsigned int i)
 {
+	if (bitmap == NULL)
+		return -AML_EINVAL;
 	if (i >= AML_BITMAP_MAX)
-		return -1;
+		return -AML_EINVAL;
 	return (bitmap->mask[AML_BITMAP_NTH(i)] &
 		(1UL << AML_BITMAP_ITH(i))) > 0UL;
 }
 
 int aml_bitmap_set(struct aml_bitmap *bitmap, const unsigned int i)
 {
+	if (bitmap == NULL)
+		return -AML_EINVAL;
 	if (i >= AML_BITMAP_MAX)
-		return -1;
+		return -AML_EINVAL;
 	bitmap->mask[AML_BITMAP_NTH(i)] |= (1UL << AML_BITMAP_ITH(i));
 	return 0;
 }
 
 int aml_bitmap_isequal(const struct aml_bitmap *a, const struct aml_bitmap *b)
 {
+	if (a == NULL || b == NULL)
+		return -AML_EINVAL;
 	for (unsigned int i = 0; i < AML_BITMAP_SIZE; i++)
 		if (a->mask[i] != b->mask[i])
 			return 0;
@@ -102,8 +118,10 @@ int aml_bitmap_isequal(const struct aml_bitmap *a, const struct aml_bitmap *b)
 
 int aml_bitmap_clear(struct aml_bitmap *bitmap, const unsigned int i)
 {
+	if (bitmap == NULL)
+		return -AML_EINVAL;
 	if (i >= AML_BITMAP_MAX)
-		return -1;
+		return -AML_EINVAL;
 	bitmap->mask[AML_BITMAP_NTH(i)] &= ~(1UL << AML_BITMAP_ITH(i));
 	return 0;
 }
@@ -111,8 +129,10 @@ int aml_bitmap_clear(struct aml_bitmap *bitmap, const unsigned int i)
 int aml_bitmap_set_range(struct aml_bitmap *bitmap,
 			 const unsigned int i, const unsigned int ii)
 {
+	if (bitmap == NULL)
+		return -AML_EINVAL;
 	if (i >= AML_BITMAP_MAX || ii >= AML_BITMAP_MAX || i > ii)
-		return -1;
+		return -AML_EINVAL;
 	if (i == ii)
 		return aml_bitmap_set(bitmap, i);
 
@@ -136,8 +156,10 @@ int aml_bitmap_set_range(struct aml_bitmap *bitmap,
 int aml_bitmap_clear_range(struct aml_bitmap *bitmap,
 			   const unsigned int i, const unsigned int ii)
 {
+	if (bitmap == NULL)
+		return -AML_EINVAL;
 	if (i >= AML_BITMAP_MAX || ii >= AML_BITMAP_MAX || i > ii)
-		return -1;
+		return -AML_EINVAL;
 	if (i == ii)
 		return aml_bitmap_clear(bitmap, i);
 
@@ -158,15 +180,18 @@ int aml_bitmap_clear_range(struct aml_bitmap *bitmap,
 	return 0;
 }
 
-unsigned long aml_bitmap_nset(const struct aml_bitmap *bitmap)
+int aml_bitmap_nset(const struct aml_bitmap *bitmap)
 {
-	unsigned long i, b, n;
 	unsigned long test = 1UL;
-	unsigned long nset = 0;
+	int nset = 0;
 
-	for (n = 0; n < AML_BITMAP_SIZE; n++) {
-		b = bitmap->mask[n];
-		for (i = 0; i < AML_BITMAP_NBITS; i++) {
+	if (bitmap == NULL)
+		return -AML_EINVAL;
+
+	for (int n = 0; n < AML_BITMAP_SIZE; n++) {
+		unsigned long b = bitmap->mask[n];
+
+		for (int i = 0; i < AML_BITMAP_NBITS; i++) {
 			nset += b & test ? 1 : 0;
 			b = b >> 1;
 		}
@@ -176,11 +201,12 @@ unsigned long aml_bitmap_nset(const struct aml_bitmap *bitmap)
 
 int aml_bitmap_last(const struct aml_bitmap *bitmap)
 {
-	if (bitmap == NULL)
-		return -1;
 	int n;
 	unsigned int i = 0;
 
+	if (bitmap == NULL)
+		return -AML_EINVAL;
+
 	for (n = AML_BITMAP_SIZE - 1; n >= 0 && bitmap->mask[n] == 0; n--)
 		;
 
@@ -301,6 +327,9 @@ int aml_bitmap_from_string(struct aml_bitmap *bitmap, const char *bitmap_str)
 
 int aml_bitmap_create(struct aml_bitmap **map)
 {
+	if (map == NULL)
+		return -AML_EINVAL;
+
 	struct aml_bitmap *b = calloc(1, sizeof(struct aml_bitmap));
 
 	if (b == NULL) {
@@ -319,6 +348,9 @@ int aml_bitmap_create(struct aml_bitmap **map)
  **/
 void aml_bitmap_destroy(struct aml_bitmap **map)
 {
+	if (map == NULL)
+		return;
+
 	free(*map);
 	*map = NULL;
 }

tests/utils/test_bitmap.c

@@ -11,108 +11,163 @@
 #include "aml.h"
 #include <assert.h>
 
-void test_bitmap_fill(){
-	unsigned long i;
-	struct aml_bitmap b;
-	aml_bitmap_fill(&b);
-	for(i = 0; i < AML_BITMAP_MAX; i++)
-		assert(aml_bitmap_isset(&b, i));
-	assert(aml_bitmap_nset(&b) == AML_BITMAP_MAX);
-}
+/* set of bits that are interesting to test. In increasing order by design, for
+ * the range tests.
+ * - first one
+ * - middle of a backing element
+ * - last one in the middle of the backing array
+ * - middle of a backing element
+ * - first one in the middle of the backing array
+ * - middle of a backing element
+ * - last one
+ */
+#define NTESTS 7
+#define TESTS_IDX {0, AML_BITMAP_NBITS + 16, \
+	(2*AML_BITMAP_NBITS) - 1, (2*AML_BITMAP_NBITS) + 16, \
+	3*AML_BITMAP_NBITS, (3*AML_BITMAP_NBITS) + 16, AML_BITMAP_MAX - 1}
+#define TESTS_OFF {0, 1, 1, 2, 3, 3, AML_BITMAP_SIZE - 1}
 
-void test_bitmap_zero(){
-	unsigned long i;
-	struct aml_bitmap b;
-	aml_bitmap_zero(&b);
-	for(i = 0; i < AML_BITMAP_MAX; i++)
-		assert(!aml_bitmap_isset(&b, i));
-	assert(aml_bitmap_nset(&b) == 0);
-}
+int main(void)
+{
+	struct aml_bitmap *bitmap;
+	int idx[NTESTS] = TESTS_IDX;
+	int off[NTESTS] = TESTS_OFF;
+
+	/* create/destroy should fail on NULL */
+	assert(aml_bitmap_create(NULL) == -AML_EINVAL);
+	aml_bitmap_destroy(NULL);
+
+	/* create should succeed with a pointer */
+	assert(aml_bitmap_create(&bitmap) == AML_SUCCESS);
+	assert(bitmap != NULL);
+
+	/* access the mask just to be sure */
+	assert(bitmap->mask[0] == 0);
+	aml_bitmap_destroy(&bitmap);
+	assert(bitmap == NULL);
 
-void test_bitmap_set(){
-	unsigned long i,j;
-	struct aml_bitmap b;
+	/* keep a bitmap on for the rest of the tests */
+	assert(aml_bitmap_create(&bitmap) == AML_SUCCESS);
 
-	aml_bitmap_zero(&b);
-	for(i = 0; i < AML_BITMAP_MAX; i++){
-		aml_bitmap_set(&b, i);
-		assert(aml_bitmap_isset(&b, i));
+	/* zero should work */
+	assert(aml_bitmap_zero(NULL) == -AML_EINVAL);
+	memset(bitmap->mask, 255, AML_BITMAP_BYTES);
+	assert(aml_bitmap_zero(bitmap) == AML_SUCCESS);
+	for (size_t i = 0; i < AML_BITMAP_SIZE; i++)
+		assert(bitmap->mask[i] == 0);
 
-		for(j = 0; j < i; j++)
-			assert(!aml_bitmap_isset(&b, j));
-		for(j = i+1; j < AML_BITMAP_MAX; j++)
-			assert(!aml_bitmap_isset(&b, j));
+	/* iszero/isfull (1) */
+	assert(aml_bitmap_iszero(NULL) == -AML_EINVAL);
+	assert(aml_bitmap_isfull(NULL) == -AML_EINVAL);
+	assert(aml_bitmap_iszero(bitmap));
+	assert(!aml_bitmap_isfull(bitmap));
 
-		assert(aml_bitmap_nset(&b) == 1);
-		aml_bitmap_clear(&b, i);
-		assert(!aml_bitmap_isset(&b, i));
+	/* nset (1) */
+	assert(aml_bitmap_nset(bitmap) == 0);
+
+	/* fill should work */
+	assert(aml_bitmap_fill(NULL) == -AML_EINVAL);
+	assert(aml_bitmap_fill(bitmap) == AML_SUCCESS);
+	for (size_t i = 0; i < AML_BITMAP_SIZE; i++)
+		assert(bitmap->mask[i] != 0);
+	assert(!aml_bitmap_iszero(bitmap));
+	assert(aml_bitmap_isfull(bitmap));
+
+	/* nset (2) */
+	assert(aml_bitmap_nset(bitmap) == AML_BITMAP_MAX);
+
+	/* set / clear */
+	aml_bitmap_zero(bitmap);
+	assert(aml_bitmap_set(NULL, 0) == -AML_EINVAL);
+	assert(aml_bitmap_set(NULL, AML_BITMAP_MAX) == -AML_EINVAL);
+	for (int i = 0; i < NTESTS; i++) {
+		aml_bitmap_zero(bitmap);
+		assert(aml_bitmap_set(bitmap, idx[i]) == AML_SUCCESS);
+		assert(bitmap->mask[off[i]] != 0);
+		/* nset (3) */
+		assert(aml_bitmap_nset(bitmap) == 1);
+		/* clear that offset and check for zero */
+		bitmap->mask[off[i]] = 0;
+		assert(aml_bitmap_iszero(bitmap));
 	}
-}
 
-void test_bitmap_clear(){
-	unsigned long i,j;
-	struct aml_bitmap b;
-	aml_bitmap_fill(&b);
-	for(i = 0; i < AML_BITMAP_MAX; i++){
-		aml_bitmap_clear(&b, i);
-		assert(!aml_bitmap_isset(&b, i));
-
-		for(j = 0; j < i; j++)
-			assert(aml_bitmap_isset(&b, j));
-		for(j = i+1; j < AML_BITMAP_MAX; j++)
-			assert(aml_bitmap_isset(&b, j));
-
-		assert(aml_bitmap_nset(&b) == (AML_BITMAP_MAX-1));
-		aml_bitmap_set(&b, i);
-		assert(aml_bitmap_isset(&b, i));
+	/* same logic for clear */
+	assert(aml_bitmap_clear(NULL, 0) == -AML_EINVAL);
+	assert(aml_bitmap_clear(NULL, AML_BITMAP_MAX) == -AML_EINVAL);
+	for (int i = 0; i < NTESTS; i++) {
+		aml_bitmap_fill(bitmap);
+		assert(aml_bitmap_clear(bitmap, idx[i]) == AML_SUCCESS);
+		assert(bitmap->mask[off[i]] != ~0UL);
+		/* nset (4) */
+		assert(aml_bitmap_nset(bitmap) == AML_BITMAP_MAX - 1);
+		/* set that offset and check for full */
+		bitmap->mask[off[i]] = ~0UL;
+		assert(aml_bitmap_isfull(bitmap));
 	}
-}
 
-void test_bitmap_set_range(){
-	unsigned long i, ii, j;
-	struct aml_bitmap b;
-	aml_bitmap_zero(&b);
-	for(i = 0; i < AML_BITMAP_MAX; i++){
-		for(ii = i+1; ii < AML_BITMAP_MAX; ii++){
-			assert(aml_bitmap_set_range(&b, i, ii) == 0);
-			assert(aml_bitmap_nset(&b) == (1 + ii - i));
-			for(j = 0; j < i; j++)
-				assert(!aml_bitmap_isset(&b, j));
-			for(j = i; j <= ii; j++)
-				assert(aml_bitmap_isset(&b, j));
-			for(j = ii+1; j < AML_BITMAP_MAX; j++)
-				assert(!aml_bitmap_isset(&b, j));
-			aml_bitmap_zero(&b);
-		}
+	/* idem for isset */
+	assert(aml_bitmap_isset(NULL, 0) == -AML_EINVAL);
+	assert(aml_bitmap_isset(bitmap, AML_BITMAP_MAX) == -AML_EINVAL);
+	aml_bitmap_zero(bitmap);
+	for (int i = 1; i < NTESTS; i++) {
+		aml_bitmap_set(bitmap, idx[i]);
+		assert(aml_bitmap_isset(bitmap, idx[i]));
+		assert(!aml_bitmap_isset(bitmap, idx[i-1]));
+		aml_bitmap_clear(bitmap, idx[i]);
+		assert(!aml_bitmap_isset(bitmap, idx[i]));
+		assert(!aml_bitmap_isset(bitmap, idx[i-1]));
 	}
-}
 
-void test_bitmap_clear_range(){
-	unsigned long i, ii, j;
-	struct aml_bitmap b;
-	aml_bitmap_fill(&b);
-	for(i = 0; i < AML_BITMAP_MAX; i++){
-		for(ii = i+1; ii < AML_BITMAP_MAX; ii++){
-			assert(aml_bitmap_clear_range(&b, i, ii) == 0);
-			assert(aml_bitmap_nset(&b) == (AML_BITMAP_MAX-ii+i-1));
-			for(j = 0; j < i; j++)
-				assert(aml_bitmap_isset(&b, j));
-			for(j = i; j <= ii; j++)
-				assert(!aml_bitmap_isset(&b, j));
-			for(j = ii+1; j < AML_BITMAP_MAX; j++)
-				assert(aml_bitmap_isset(&b, j));
-			aml_bitmap_fill(&b);
+	/* bad input for ranges (out-of-bounds, reverse indexes) */
+	assert(aml_bitmap_set_range(NULL, 0, 1) == -AML_EINVAL);
+	assert(aml_bitmap_set_range(bitmap, 0, AML_BITMAP_MAX) == -AML_EINVAL);
+	assert(aml_bitmap_set_range(bitmap, AML_BITMAP_MAX,
+				    AML_BITMAP_MAX+1) == -AML_EINVAL);
+	assert(aml_bitmap_set_range(bitmap, AML_BITMAP_MAX, 0) == -AML_EINVAL);
+	assert(aml_bitmap_set_range(bitmap, 1, 0) == -AML_EINVAL);
+
+	for (int i = 0; i < NTESTS; i++) {
+		for (int j = i; j < NTESTS; j++) {
+			aml_bitmap_zero(bitmap);
+			assert(aml_bitmap_set_range(bitmap, idx[i], idx[j]) ==
+			       AML_SUCCESS);
+			for (int k = 0; k < idx[i]; k++)
+				assert(!aml_bitmap_isset(bitmap, k));
+			for (int k = idx[i]; k <= idx[j]; k++)
+				assert(aml_bitmap_isset(bitmap, k));
+			for (int k = idx[j]+1; k < AML_BITMAP_MAX; k++)
+				assert(!aml_bitmap_isset(bitmap, k));
+			/* nset (5) */
+			assert(aml_bitmap_nset(bitmap) == 1 + idx[j] - idx[i]);
 		}
 	}
-}
 
-int main(){
-	test_bitmap_fill();
-	test_bitmap_zero();
-	test_bitmap_set();
-	test_bitmap_clear();
-	test_bitmap_set_range();
-	test_bitmap_clear_range();
+	/* same logic for clear */
+	assert(aml_bitmap_clear_range(NULL, 0, 1) == -AML_EINVAL);
+	assert(aml_bitmap_clear_range(bitmap, 0, AML_BITMAP_MAX) ==
+	       -AML_EINVAL);
+	assert(aml_bitmap_clear_range(bitmap, AML_BITMAP_MAX,
+				    AML_BITMAP_MAX+1) == -AML_EINVAL);
+	assert(aml_bitmap_clear_range(bitmap, AML_BITMAP_MAX, 0) ==
+	       -AML_EINVAL);
+	assert(aml_bitmap_clear_range(bitmap, 1, 0) == -AML_EINVAL);
+
+	for (int i = 0; i < NTESTS; i++) {
+		for (int j = i; j < NTESTS; j++) {
+			aml_bitmap_fill(bitmap);
+			assert(aml_bitmap_clear_range(bitmap, idx[i], idx[j]) ==
+			       AML_SUCCESS);
+			for (int k = 0; k < idx[i]; k++)
+				assert(aml_bitmap_isset(bitmap, k));
+			for (int k = idx[i]; k <= idx[j]; k++)
+				assert(!aml_bitmap_isset(bitmap, k));
+			for (int k = idx[j]+1; k < AML_BITMAP_MAX; k++)
+				assert(aml_bitmap_isset(bitmap, k));
+			assert(aml_bitmap_nset(bitmap) ==
+			       AML_BITMAP_MAX - idx[j] + idx[i] - 1);
+		}
+	}
+	aml_bitmap_destroy(&bitmap);
 	return 0;
 }