From d70a25c67910d58d4ff0f12edb8e106ce94a6d59 Mon Sep 17 00:00:00 2001 From: Swann Perarnau Date: Tue, 13 Aug 2019 16:39:55 -0500 Subject: [PATCH] [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 | 10 +- src/utils/bitmap.c | 62 +++++++--- tests/utils/test_bitmap.c | 231 +++++++++++++++++++++++-------------- 3 files changed, 195 insertions(+), 108 deletions(-) diff --git a/include/aml/utils/bitmap.h b/include/aml/utils/bitmap.h index 796480f..006b5e7 100644 --- a/include/aml/utils/bitmap.h +++ b/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. diff --git a/src/utils/bitmap.c b/src/utils/bitmap.c index f282e3f..aba4115 100644 --- a/src/utils/bitmap.c +++ b/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; } diff --git a/tests/utils/test_bitmap.c b/tests/utils/test_bitmap.c index fde289c..b917edf 100644 --- a/tests/utils/test_bitmap.c +++ b/tests/utils/test_bitmap.c @@ -11,108 +11,163 @@ #include "aml.h" #include -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; } -- 2.26.2