Rework BitVector_Next_Hit - Avoid overflow in pointer arithmetic. - Fix conversion warnings. - Relax int32_t range check. - Simplify. - Fix and add test.
Project: http://git-wip-us.apache.org/repos/asf/lucy/repo Commit: http://git-wip-us.apache.org/repos/asf/lucy/commit/c0e4d81d Tree: http://git-wip-us.apache.org/repos/asf/lucy/tree/c0e4d81d Diff: http://git-wip-us.apache.org/repos/asf/lucy/diff/c0e4d81d Branch: refs/heads/master Commit: c0e4d81d8ce4d4dc928d9f641321c8f8d7b78223 Parents: 745a278 Author: Nick Wellnhofer <[email protected]> Authored: Wed Jul 6 19:49:30 2016 +0200 Committer: Nick Wellnhofer <[email protected]> Committed: Wed Jul 6 20:23:30 2016 +0200 ---------------------------------------------------------------------- core/Lucy/Object/BitVector.c | 29 +++++++++++++---------------- core/Lucy/Test/Object/TestBitVector.c | 6 ++++-- 2 files changed, 17 insertions(+), 18 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/lucy/blob/c0e4d81d/core/Lucy/Object/BitVector.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Object/BitVector.c b/core/Lucy/Object/BitVector.c index 202475a..ba07b18 100644 --- a/core/Lucy/Object/BitVector.c +++ b/core/Lucy/Object/BitVector.c @@ -193,36 +193,33 @@ S_first_bit_in_nonzero_byte(uint8_t num) { int32_t BitVec_Next_Hit_IMP(BitVector *self, size_t tick) { BitVectorIVARS *const ivars = BitVec_IVARS(self); - size_t byte_size = SI_octet_size(ivars->cap); - uint8_t *const limit = ivars->bits + byte_size; - uint8_t *ptr = ivars->bits + (tick >> 3); - if (ivars->cap > INT32_MAX / 8) { + if (ivars->cap > INT32_MAX) { THROW(ERR, "Capacity too large for Next_Hit: %u64", (uint64_t)ivars->cap); } - - if (ptr >= limit) { + if (tick >= ivars->cap) { return -1; } - else if (*ptr != 0) { + + size_t byte_size = SI_octet_size(ivars->cap); + uint8_t *const limit = ivars->bits + byte_size; + uint8_t *ptr = ivars->bits + (tick >> 3); + + if (*ptr != 0) { // Special case the first byte. - const int32_t base = (ptr - ivars->bits) * 8; - const int32_t min_sub_tick = tick & 0x7; - unsigned int byte = *ptr >> min_sub_tick; + size_t min_sub_tick = tick & 0x7; + uint8_t byte = (uint8_t)(*ptr >> min_sub_tick); if (byte) { - const int32_t candidate - = base + min_sub_tick + S_first_bit_in_nonzero_byte(byte); - return candidate < (int32_t)ivars->cap ? candidate : -1; + return (int32_t)tick + S_first_bit_in_nonzero_byte(byte); } } for (ptr++ ; ptr < limit; ptr++) { if (*ptr != 0) { // There's a non-zero bit in this byte. - const int32_t base = (ptr - ivars->bits) * 8; - const int32_t candidate = base + S_first_bit_in_nonzero_byte(*ptr); - return candidate < (int32_t)ivars->cap ? candidate : -1; + int32_t base = (int32_t)((ptr - ivars->bits) * 8); + return base + S_first_bit_in_nonzero_byte(*ptr); } } http://git-wip-us.apache.org/repos/asf/lucy/blob/c0e4d81d/core/Lucy/Test/Object/TestBitVector.c ---------------------------------------------------------------------- diff --git a/core/Lucy/Test/Object/TestBitVector.c b/core/Lucy/Test/Object/TestBitVector.c index d6080b7..03b3c57 100644 --- a/core/Lucy/Test/Object/TestBitVector.c +++ b/core/Lucy/Test/Object/TestBitVector.c @@ -329,7 +329,7 @@ test_Next_Hit(TestBatchRunner *runner) { BitVec_Set(bit_vec, (size_t)i); TEST_INT_EQ(runner, BitVec_Next_Hit(bit_vec, 0), i, "Next_Hit for 0 is %d", i); - TEST_INT_EQ(runner, BitVec_Next_Hit(bit_vec, 0), i, + TEST_INT_EQ(runner, BitVec_Next_Hit(bit_vec, 1), i, "Next_Hit for 1 is %d", i); for (int probe = 15; probe <= i; probe++) { TEST_INT_EQ(runner, BitVec_Next_Hit(bit_vec, (size_t)probe), i, @@ -339,6 +339,8 @@ test_Next_Hit(TestBatchRunner *runner) { TEST_INT_EQ(runner, BitVec_Next_Hit(bit_vec, (size_t)probe), -1, "no Next_Hit for %d when max is %d", probe, i); } + TEST_INT_EQ(runner, BitVec_Next_Hit(bit_vec, INT32_MAX), -1, + "no Next_Hit for INT32_MAX when max is %d", i); DECREF(bit_vec); } } @@ -429,7 +431,7 @@ test_off_by_one_error() { void TestBitVector_Run_IMP(TestBitVector *self, TestBatchRunner *runner) { - TestBatchRunner_Plan(runner, (TestBatch*)self, 1029); + TestBatchRunner_Plan(runner, (TestBatch*)self, 1039); test_Set_and_Get(runner); test_Flip(runner); test_Flip_Block_ascending(runner);
