Skip to content

Commit ffa4b35

Browse files
committed
Add unit tests and fix bugs.
1 parent ca75865 commit ffa4b35

File tree

9 files changed

+346
-24
lines changed

9 files changed

+346
-24
lines changed

README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -821,8 +821,8 @@ for (uint32_t i = 100000; i < 500000; i+= 100) {
821821
roaring_bitmap_add_range(r1, 500000, 600000);
822822
823823
// Convert a range to boolean array
824-
size_t range_start = 50; // Start from the 50th element
825-
size_t range_end = 1000; // End at the 1000th element (not included)
824+
uint32_t range_start = 50; // Start from the 50th element
825+
uint32_t range_end = 1000; // End at the 1000th element (not included)
826826
bool *bool_array = malloc((range_end - range_start) * sizeof(bool));
827827
828828
// Convert range to boolean array

include/roaring/roaring.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -572,7 +572,7 @@ bool roaring_bitmap_to_bitset(const roaring_bitmap_t *r, bitset_t *bitset);
572572
* many values were actually read.
573573
*/
574574
void roaring_bitmap_range_bool_array(const roaring_bitmap_t *r,
575-
size_t range_start, size_t range_end,
575+
uint32_t range_start, uint32_t range_end,
576576
bool *ans);
577577
/**
578578
* Convert the bitmap to a sorted array from `offset` by `limit`, output in

src/containers/bitset.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,7 @@ bool bitset_container_iterator_read_into_bool(const bitset_container_t *bc,
255255
// If max_value is not NULL, get the wordindex of the max_value.
256256
if (max_value != NULL) {
257257
max_wordindex = *max_value / 64;
258-
if (max_wordindex > BITSET_CONTAINER_SIZE_IN_WORDS) {
259-
max_wordindex = BITSET_CONTAINER_SIZE_IN_WORDS - 1;
260-
}
258+
assert(max_wordindex < BITSET_CONTAINER_SIZE_IN_WORDS);
261259
}
262260
uint32_t wordindex = it->index / 64;
263261
uint64_t word = bc->words[wordindex] & (UINT64_MAX << (it->index % 64));
@@ -266,8 +264,8 @@ bool bitset_container_iterator_read_into_bool(const bitset_container_t *bc,
266264
while (wordindex < max_wordindex) {
267265
// TODO: SIMD optimization
268266
while (word != 0) {
269-
uint16_t value = wordindex * 64 + roaring_trailing_zeroes(word);
270-
buf[value - initial_value] = true;
267+
*value_out = wordindex * 64 + roaring_trailing_zeroes(word);
268+
buf[*value_out - initial_value] = true;
271269
word = word & (word - 1);
272270
}
273271
wordindex++;
@@ -280,15 +278,17 @@ bool bitset_container_iterator_read_into_bool(const bitset_container_t *bc,
280278
if (max_value == NULL) return false;
281279
// Process the last word (which is at max_wordindex)
282280
while (word != 0) {
283-
uint16_t value = wordindex * 64 + roaring_trailing_zeroes(word);
284-
if (value >= *max_value) {
285-
it->index = value;
286-
*value_out = value;
281+
*value_out = wordindex * 64 + roaring_trailing_zeroes(word);
282+
if (*value_out >= *max_value) {
283+
it->index = *value_out;
287284
return true;
288285
}
289-
buf[value - initial_value] = true;
286+
buf[*value_out - initial_value] = true;
290287
word = word & (word - 1);
291288
}
289+
// If max_value is not NULL, its wordindex must be less than
290+
// BITSET_CONTAINER_SIZE_IN_WORDS. So if reach this line, the bitset must be
291+
// drained.
292292
return false;
293293
}
294294

src/containers/run.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1154,13 +1154,19 @@ bool run_container_iterator_read_into_bool(const run_container_t *rc,
11541154

11551155
// Start from current value if we're in the middle of a run
11561156
uint16_t start = (*value_out >= run_start) ? *value_out : run_start;
1157-
if (*max_value < run_end + 1) {
1157+
// max_value .. [start .. run_end]
1158+
if (*max_value <= start) {
1159+
*value_out = start;
1160+
return true;
1161+
}
1162+
// [start .. max_value .. run_end]
1163+
if (*max_value <= run_end) {
11581164
memset(buf + start - initial_value, true, *max_value - start);
11591165
*value_out = *max_value;
11601166
return true;
1161-
} else {
1162-
memset(buf + start - initial_value, true, run_end - start + 1);
11631167
}
1168+
// [start .. run_end] .. max_value
1169+
memset(buf + start - initial_value, true, run_end - start + 1);
11641170
*value_out = run_end;
11651171
it->index++;
11661172
}

src/roaring.c

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1883,7 +1883,7 @@ uint32_t roaring_uint32_iterator_read(roaring_uint32_iterator_t *it,
18831883
void roaring_uint32_iterator_read_into_bool(roaring_uint32_iterator_t *it,
18841884
bool *buf, uint32_t max_value) {
18851885
uint32_t initial_value = it->current_value;
1886-
uint32_t highbits_of_max_value = (max_value >> 16);
1886+
uint32_t highbits_of_max_value = (max_value & 0xFFFF0000);
18871887
uint16_t lowbits_of_max_value = (uint16_t)max_value;
18881888
while (it->has_value && it->current_value < max_value) {
18891889
buf += (it->current_value - initial_value);
@@ -3510,12 +3510,13 @@ bool roaring_bitmap_to_bitset(const roaring_bitmap_t *r, bitset_t *bitset) {
35103510
}
35113511

35123512
void roaring_bitmap_range_bool_array(const roaring_bitmap_t *r,
3513-
size_t range_start, size_t range_end,
3513+
uint32_t range_start, uint32_t range_end,
35143514
bool *ans) {
35153515
roaring_uint32_iterator_t it;
35163516
roaring_iterator_init(r, &it);
3517-
roaring_uint32_iterator_skip(&it, range_start);
3518-
roaring_uint32_iterator_read_into_bool(&it, ans, range_end);
3517+
if (!roaring_uint32_iterator_move_equalorlarger(&it, range_start)) return;
3518+
roaring_uint32_iterator_read_into_bool(
3519+
&it, ans + it.current_value - range_start, range_end);
35193520
}
35203521

35213522
#ifdef __cplusplus

tests/array_container_unit.c

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
#include <roaring/containers/array.h>
1111
#include <roaring/containers/bitset.h>
12+
#include <roaring/containers/containers.h>
1213
#include <roaring/containers/mixed_equal.h>
1314
#include <roaring/misc/configreport.h>
1415

@@ -193,6 +194,71 @@ DEFINE_TEST(capacity_test) {
193194
array_container_free(array);
194195
}
195196

197+
DEFINE_TEST(iterator_read_into_bool_test) {
198+
array_container_t* A = array_container_create();
199+
assert_non_null(A);
200+
201+
// Variables to use.
202+
uint16_t initial_value = 0;
203+
uint16_t value_out = 0;
204+
uint16_t max_value = 0;
205+
roaring_container_iterator_t it;
206+
const uint16_t max_elements = 600;
207+
bool* ans_array = (bool*)calloc(max_elements, sizeof(bool));
208+
bool* bool_array;
209+
210+
// Add values with gaps
211+
for (uint16_t i = 100; i < 200; i += 5) {
212+
array_container_add(A, i);
213+
ans_array[i] = true;
214+
}
215+
for (uint16_t i = 500; i < max_elements; i += 3) {
216+
array_container_add(A, i);
217+
ans_array[i] = true;
218+
}
219+
220+
// Test 1: Read without max_value (read all)
221+
it = container_init_iterator(A, ARRAY_CONTAINER_TYPE, &initial_value);
222+
bool_array = (bool*)calloc(max_elements - initial_value, sizeof(bool));
223+
value_out = initial_value;
224+
bool has_more = array_container_iterator_read_into_bool(A, &it, bool_array,
225+
NULL, &value_out);
226+
assert_false(has_more); // Should read all values
227+
assert_true(memcmp(ans_array + initial_value, bool_array,
228+
max_elements - initial_value) == 0);
229+
free(bool_array);
230+
231+
// Test 2: Read with max_value
232+
it = container_init_iterator(A, ARRAY_CONTAINER_TYPE, &initial_value);
233+
max_value = 300;
234+
bool_array = (bool*)calloc(max_value - initial_value, sizeof(bool));
235+
value_out = initial_value;
236+
has_more = array_container_iterator_read_into_bool(A, &it, bool_array,
237+
&max_value, &value_out);
238+
assert_true(has_more);
239+
assert_true(memcmp(ans_array + initial_value, bool_array,
240+
max_value - initial_value) == 0);
241+
free(bool_array);
242+
243+
// Test 3: Read from middle with max_value
244+
uint32_t consumed;
245+
it = container_init_iterator(A, ARRAY_CONTAINER_TYPE, &initial_value);
246+
container_iterator_skip(A, ARRAY_CONTAINER_TYPE, &it, 10, &consumed,
247+
&initial_value);
248+
max_value = 550;
249+
bool_array = (bool*)calloc(max_value - initial_value, sizeof(bool));
250+
value_out = initial_value;
251+
has_more = array_container_iterator_read_into_bool(A, &it, bool_array,
252+
&max_value, &value_out);
253+
assert_true(has_more);
254+
assert_true(memcmp(ans_array + initial_value, bool_array,
255+
max_value - initial_value) == 0);
256+
free(bool_array);
257+
258+
array_container_free(A);
259+
free(ans_array);
260+
}
261+
196262
/* This is a fixed-increment version of Java 8's SplittableRandom generator
197263
See http://dx.doi.org/10.1145/2714064.2660195 and
198264
http://docs.oracle.com/javase/8/docs/api/java/util/SplittableRandom.html */
@@ -374,7 +440,8 @@ int main() {
374440
cmocka_unit_test(and_or_test),
375441
cmocka_unit_test(to_uint32_array_test),
376442
cmocka_unit_test(select_test),
377-
cmocka_unit_test(capacity_test)};
443+
cmocka_unit_test(capacity_test),
444+
cmocka_unit_test(iterator_read_into_bool_test)};
378445

379446
return cmocka_run_group_tests(tests, NULL, NULL);
380447
}

tests/bitset_container_unit.c

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@
66
#include <stdint.h>
77
#include <stdio.h>
88
#include <stdlib.h>
9+
#include <string.h>
910

1011
#include <roaring/bitset_util.h>
1112
#include <roaring/containers/bitset.h>
13+
#include <roaring/containers/containers.h>
1214
#include <roaring/misc/configreport.h>
1315

1416
#ifdef __cplusplus // stronger type checking errors if C built in C++ mode
@@ -314,6 +316,72 @@ DEFINE_TEST(select_test) {
314316
bitset_container_free(B);
315317
}
316318

319+
DEFINE_TEST(iterator_read_into_bool_test) {
320+
bitset_container_t* B = bitset_container_create();
321+
assert_non_null(B);
322+
323+
// Variables to use.
324+
uint16_t initial_value = 0;
325+
uint16_t value_out = 0;
326+
uint16_t max_value = 0;
327+
roaring_container_iterator_t it;
328+
const uint16_t max_elements = 600;
329+
bool* ans_array = (bool*)calloc(max_elements, sizeof(bool));
330+
bool* bool_array;
331+
332+
// Add values with gaps
333+
for (uint16_t i = 100; i < 200; i += 3) {
334+
bitset_container_set(B, i);
335+
ans_array[i] = true;
336+
}
337+
for (uint16_t i = 500; i < max_elements; i += 2) {
338+
bitset_container_set(B, i);
339+
ans_array[i] = true;
340+
}
341+
B->cardinality = bitset_container_compute_cardinality(B);
342+
343+
// Test 1: Read without max_value (read all)
344+
it = container_init_iterator(B, BITSET_CONTAINER_TYPE, &initial_value);
345+
value_out = initial_value;
346+
bool_array = (bool*)calloc(500, sizeof(bool));
347+
bool has_more = bitset_container_iterator_read_into_bool(B, &it, bool_array,
348+
NULL, &value_out);
349+
assert_false(has_more); // Should read all values
350+
assert_true(memcmp(ans_array + initial_value, bool_array,
351+
max_elements - initial_value) == 0);
352+
free(bool_array);
353+
354+
// Test 2: Read with max_value
355+
it = container_init_iterator(B, BITSET_CONTAINER_TYPE, &initial_value);
356+
max_value = 300;
357+
bool_array = (bool*)calloc(max_value - initial_value, sizeof(bool));
358+
value_out = initial_value;
359+
has_more = bitset_container_iterator_read_into_bool(B, &it, bool_array,
360+
&max_value, &value_out);
361+
assert_true(has_more);
362+
assert_true(memcmp(ans_array + initial_value, bool_array,
363+
(max_value - initial_value) * sizeof(bool)) == 0);
364+
free(bool_array);
365+
366+
// Test 3: Read from middle with max_value
367+
uint32_t consumed;
368+
it = container_init_iterator(B, BITSET_CONTAINER_TYPE, &initial_value);
369+
container_iterator_skip(B, BITSET_CONTAINER_TYPE, &it, 10, &consumed,
370+
&initial_value);
371+
max_value = 550;
372+
bool_array = (bool*)calloc(max_value - initial_value, sizeof(bool));
373+
value_out = initial_value;
374+
has_more = bitset_container_iterator_read_into_bool(B, &it, bool_array,
375+
&max_value, &value_out);
376+
assert_true(has_more);
377+
assert_true(memcmp(ans_array + initial_value, bool_array,
378+
(max_value - initial_value) * sizeof(bool)) == 0);
379+
free(bool_array);
380+
381+
bitset_container_free(B);
382+
free(ans_array);
383+
}
384+
317385
int main() {
318386
const struct CMUnitTest tests[] = {
319387
cmocka_unit_test(hamming_test),
@@ -326,6 +394,7 @@ int main() {
326394
cmocka_unit_test(to_uint32_array_test),
327395
cmocka_unit_test(select_test),
328396
cmocka_unit_test(test_bitset_compute_cardinality),
397+
cmocka_unit_test(iterator_read_into_bool_test),
329398
};
330399

331400
return cmocka_run_group_tests(tests, NULL, NULL);

tests/run_container_unit.c

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#include <stdio.h>
88
#include <stdlib.h>
99

10+
#include <roaring/containers/containers.h>
1011
#include <roaring/containers/run.h>
1112
#include <roaring/misc/configreport.h>
1213

@@ -216,13 +217,82 @@ DEFINE_TEST(remove_range_test) {
216217
run_container_free(run);
217218
}
218219

220+
DEFINE_TEST(iterator_read_into_bool_test) {
221+
run_container_t* R = run_container_create();
222+
assert_non_null(R);
223+
224+
// Variables to use.
225+
uint16_t initial_value = 0;
226+
uint16_t value_out = 0;
227+
uint16_t max_value = 0;
228+
roaring_container_iterator_t it;
229+
const uint16_t max_elements = 600;
230+
bool* ans_array = (bool*)calloc(max_elements, sizeof(bool));
231+
bool* bool_array;
232+
233+
// Add runs with gaps
234+
for (uint16_t i = 100; i < 200; i++) {
235+
run_container_add(R, i);
236+
ans_array[i] = true;
237+
}
238+
for (uint16_t i = 500; i < max_elements; i++) {
239+
run_container_add(R, i);
240+
ans_array[i] = true;
241+
}
242+
243+
// Test 1: Read without max_value (read all)
244+
it = container_init_iterator(R, RUN_CONTAINER_TYPE, &initial_value);
245+
bool_array = (bool*)calloc(max_elements - initial_value, sizeof(bool));
246+
value_out = initial_value;
247+
bool has_more = run_container_iterator_read_into_bool(R, &it, bool_array,
248+
NULL, &value_out);
249+
assert_false(has_more); // Should read all values
250+
assert_true(memcmp(ans_array + initial_value, bool_array,
251+
max_elements - initial_value) == 0);
252+
free(bool_array);
253+
254+
// Test 2: Read with max_value
255+
it = container_init_iterator(R, RUN_CONTAINER_TYPE, &initial_value);
256+
max_value = 300;
257+
bool_array = (bool*)calloc(max_value - initial_value, sizeof(bool));
258+
value_out = initial_value;
259+
has_more = run_container_iterator_read_into_bool(R, &it, bool_array,
260+
&max_value, &value_out);
261+
assert_true(has_more);
262+
assert_true(memcmp(ans_array + initial_value, bool_array,
263+
max_value - initial_value) == 0);
264+
free(bool_array);
265+
266+
// Test 3: Read from middle with max_value
267+
uint32_t consumed;
268+
it = container_init_iterator(R, RUN_CONTAINER_TYPE, &initial_value);
269+
container_iterator_skip(R, RUN_CONTAINER_TYPE, &it, 10, &consumed,
270+
&initial_value);
271+
max_value = 550;
272+
bool_array = (bool*)calloc(max_value - initial_value, sizeof(bool));
273+
value_out = initial_value;
274+
has_more = run_container_iterator_read_into_bool(R, &it, bool_array,
275+
&max_value, &value_out);
276+
assert_true(has_more);
277+
assert_true(memcmp(ans_array + initial_value, bool_array,
278+
max_value - initial_value) == 0);
279+
free(bool_array);
280+
281+
run_container_free(R);
282+
free(ans_array);
283+
}
284+
219285
int main() {
220286
tellmeall();
221287

222288
const struct CMUnitTest tests[] = {
223-
cmocka_unit_test(printf_test), cmocka_unit_test(add_contains_test),
224-
cmocka_unit_test(and_or_test), cmocka_unit_test(to_uint32_array_test),
225-
cmocka_unit_test(select_test), cmocka_unit_test(remove_range_test),
289+
cmocka_unit_test(printf_test),
290+
cmocka_unit_test(add_contains_test),
291+
cmocka_unit_test(and_or_test),
292+
cmocka_unit_test(to_uint32_array_test),
293+
cmocka_unit_test(select_test),
294+
cmocka_unit_test(remove_range_test),
295+
cmocka_unit_test(iterator_read_into_bool_test),
226296
};
227297

228298
return cmocka_run_group_tests(tests, NULL, NULL);

0 commit comments

Comments
 (0)