fixing another bug related to issue 538

RoaringBitmap · Jan 4, 2024 · cd3ec10 · cd3ec10
1 parent 8a1f072
commit cd3ec10
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 6 deletions.
diff --git a/src/roaring.c b/src/roaring.c
@@ -2362,7 +2362,8 @@ roaring_bitmap_t *roaring_bitmap_add_offset(const roaring_bitmap_t *bm,
  in_offset = (uint16_t)(offset - container_offset * (1 << 16));
 
  answer = roaring_bitmap_create();
- roaring_bitmap_set_copy_on_write(answer, is_cow(bm));
+ bool cow = is_cow(bm);
+ roaring_bitmap_set_copy_on_write(answer, cow);
 
  ans_ra = &answer->high_low_container;
 
@@ -2376,11 +2377,9 @@ roaring_bitmap_t *roaring_bitmap_add_offset(const roaring_bitmap_t *bm,
  if (key < 0 || key >= (1 << 16)) {
  continue;
  }
-
- ra_append_copy(ans_ra, bm_ra, (uint16_t)i, false);
+ ra_append_copy(ans_ra, bm_ra, (uint16_t)i, cow);
  ans_ra->keys[j++] = (uint16_t)key;
  }
-
  return answer;
  }
 
@@ -2403,7 +2402,6 @@ roaring_bitmap_t *roaring_bitmap_add_offset(const roaring_bitmap_t *bm,
  if (lo_ptr == NULL && hi_ptr == NULL) {
  continue;
  }
-
  c = ra_get_container_at_index(bm_ra, (uint16_t)i, &t);
  c = container_unwrap_shared(c, &t);
 

diff --git a/src/roaring_array.c b/src/roaring_array.c
@@ -218,7 +218,7 @@ void ra_append_copy(roaring_array_t *ra, const roaring_array_t *sa,
  extend_array(ra, 1);
  const int32_t pos = ra->size;
 
- // old contents is junk not needing freeing
+ // old contents is junk that does not need freeing
  ra->keys[pos] = sa->keys[index];
  // the shared container will be in two bitmaps
  if (copy_on_write) {

diff --git a/tests/toplevel_unit.c b/tests/toplevel_unit.c
@@ -4719,10 +4719,30 @@ DEFINE_TEST(issue538) {
  roaring_bitmap_free(deserialized);
 }
 
+DEFINE_TEST(issue538b) {
+ int shift = -65536;
+ roaring_bitmap_t *toshift = roaring_bitmap_from_range(131074, 131876, 1);
+ roaring_bitmap_set_copy_on_write(toshift, 1);
+ roaring_bitmap_t *toshift_copy = roaring_bitmap_copy(toshift);
+
+ roaring_bitmap_t *shifted = roaring_bitmap_add_offset(toshift, shift);
+ roaring_bitmap_equals(toshift, toshift_copy);
+
+ roaring_bitmap_t *expected = roaring_bitmap_from_range(131074 + shift, 131876 + shift, 1);
+ roaring_bitmap_set_copy_on_write(expected, 1);
+ assert_true(roaring_bitmap_get_cardinality(toshift) == roaring_bitmap_get_cardinality(expected));
+ assert_true(roaring_bitmap_equals(shifted, expected));
+
+ roaring_bitmap_free(toshift);
+ roaring_bitmap_free(shifted);
+ roaring_bitmap_free(expected);
+}
+
 int main() {
  tellmeall();
 
  const struct CMUnitTest tests[] = {
+ cmocka_unit_test(issue538b),
  cmocka_unit_test(issue538),
  cmocka_unit_test(simple_roaring_bitmap_or_many),
  cmocka_unit_test(robust_deserialization),