Caught a bug in roaring_bitmap_internal_validate #516
Conversation
tests/toplevel_unit.c
Outdated
|
|
||
| DEFINE_TEST(robust_deserialization) { | ||
| assert_true(deserialization_test(NULL, 0)); | ||
| const char* test1 = "\x3b\x30\x00\x00\x01\x00\x00\xfa\x2e\x01\x00\x00\x02\xff\xff"; |
There was a problem hiding this comment.
nit: This could be const char test1[] = "..."; assert_true(deserialization_test(test1, sizeof(test1)) to avoid the magic number "15"
tests/toplevel_unit.c
Outdated
| DEFINE_TEST(robust_deserialization) { | ||
| assert_true(deserialization_test(NULL, 0)); | ||
| const char* test1 = "\x3b\x30\x00\x00\x01\x00\x00\xfa\x2e\x01\x00\x00\x02\xff\xff"; | ||
| assert_true(deserialization_test(test1, 15)); |
There was a problem hiding this comment.
I'm presuming the data in test1 contains an overflowing run container, but this test will succeed if any of the following are true, but I think we expect it to specifically fail internal validation (the last option):
- This inconsistent bitmap doesn't deserialize at all
- This inconsistent bitmap deserializes, validates, and can be successfully added to
- This inconsistent bitmap deserializes, but fails validation
It might be better to be more specific about what type of error we expect from this.
There was a problem hiding this comment.
It might also be good to include a comment about what the contents of test1 are.
There was a problem hiding this comment.
I have added comments. We can certainly add better tests.
|
I will merge and release. |
We can miss an issue whereas the run containers overflow the 16-bit boundary.