From ca38bc1746556dd2d937de3e46a6c5112de92962 Mon Sep 17 00:00:00 2001
From: Roman Tsisyk <roman@tsisyk.com>
Date: Tue, 5 Aug 2014 18:04:20 +0400
Subject: [PATCH] Fix and optimize bitset index for case when key is empty
This patch fixes out of bounds memory access in bit_iterator_init
for size == 0. The patch also optimizes bitset/index library to skip
useless iterations over empty keys.
Closes #425
---
include/lib/bit/bit.h | 5 +++++
include/lib/bitset/index.h | 3 +--
src/lib/bitset/index.c | 31 +++++++++++++------------------
test/unit/bit.c | 17 +++++++++++++++++
test/unit/bit.result | 2 ++
5 files changed, 38 insertions(+), 20 deletions(-)
diff --git a/include/lib/bit/bit.h b/include/lib/bit/bit.h
index 00ba076ecf..575b2e846c 100644
--- a/include/lib/bit/bit.h
+++ b/include/lib/bit/bit.h
@@ -421,6 +421,11 @@ bit_iterator_init(struct bit_iterator *it, const void *data, size_t size,
it->start = (const char *) data;
it->next = it->start;
it->end = it->next + size;
+ if (bit_unlikely(size == 0)) {
+ it->word = 0;
+ return;
+ }
+
it->word_xor = set ? 0 : (ITER_UINT) -1;
it->word_base = 0;
diff --git a/include/lib/bitset/index.h b/include/lib/bitset/index.h
index fd1ed792b7..c35407494f 100644
--- a/include/lib/bitset/index.h
+++ b/include/lib/bitset/index.h
@@ -154,8 +154,7 @@ bitset_index_destroy(struct bitset_index *index);
/**
* @brief Insert (\a key, \a value) pair into \a index.
- * Only one pair with a given value can exist in the index.
- * If a pair with the same \a value exists, it is updated quietly.
+ * \a value must be unique in the index.
* This method is atomic, i.e. \a index will be in a consistent
* state after a return even in case of error.
*
diff --git a/src/lib/bitset/index.c b/src/lib/bitset/index.c
index 541ac65a92..ff51fe9ec0 100644
--- a/src/lib/bitset/index.c
+++ b/src/lib/bitset/index.c
@@ -162,7 +162,10 @@ bitset_index_insert(struct bitset_index *index, const void *key,
if (rc < 0)
return -1;
- /* if 1 then the value is new in the index */
+ assert(rc == 0); /* if 1 then the value is already exist in the index */
+ if (key_size == 0) /* optimization for empty key */
+ return 0;
+
index->rollback_buf[0] = (char) rc;
/*
@@ -188,23 +191,6 @@ bitset_index_insert(struct bitset_index *index, const void *key,
index->rollback_buf[b] = (char) rc;
}
- /* Finish here if the value is new in the index */
- if (index->rollback_buf[0] == 0)
- return 0;
-
- /*
- * Step 3: Iterate over 'unset' bits and cleanup other bitsets
- * This step is needed if the value was already existed in the index.
- * Nothing can fail here because current implementation of
- * bitset_clear never fails.
- */
- bit_iterator_init(&bit_it, key, key_size, false);
- while ((pos = bit_iterator_next(&bit_it)) != SIZE_MAX) {
- size_t b = pos + 1;
- rc = bitset_clear(index->bitsets[b], value);
- assert(rc >= 0); /* bitset_clear never fails */
- }
-
return 0;
rollback:
@@ -308,6 +294,9 @@ bitset_index_expr_all_set(struct bitset_expr *expr, const void *key,
if (bitset_expr_add_conj(expr) != 0)
return -1;
+ if (key_size == 0)
+ return 0; /* optimization for empty key */
+
struct bit_iterator bit_it;
bit_iterator_init(&bit_it, key, key_size, true);
size_t pos;
@@ -326,6 +315,9 @@ bitset_index_expr_any_set(struct bitset_expr *expr, const void *key,
{
bitset_expr_clear(expr);
+ if (key_size == 0)
+ return 0; /* optimization for empty key */
+
struct bit_iterator bit_it;
bit_iterator_init(&bit_it, key, key_size, true);
size_t pos;
@@ -351,6 +343,9 @@ bitset_index_expr_all_not_set(struct bitset_expr *expr, const void *key,
if (bitset_expr_add_param(expr, 0, false) != 0)
return -1;
+ if (key_size == 0)
+ return 0; /* optimization for empty key */
+
struct bit_iterator bit_it;
bit_iterator_init(&bit_it, key, key_size, true);
size_t pos;
diff --git a/test/unit/bit.c b/test/unit/bit.c
index a7cce03672..d3eadbea19 100644
--- a/test/unit/bit.c
+++ b/test/unit/bit.c
@@ -190,6 +190,22 @@ test_bit_iter(void)
footer();
}
+static void
+test_bit_iter_empty(void)
+{
+ header();
+
+ struct bit_iterator it;
+
+ bit_iterator_init(&it, NULL, 0, true);
+ fail_unless(bit_iterator_next(&it) == SIZE_MAX);
+
+ bit_iterator_init(&it, NULL, 0, false);
+ fail_unless(bit_iterator_next(&it) == SIZE_MAX);
+
+ footer();
+}
+
int
main(void)
{
@@ -199,4 +215,5 @@ main(void)
test_bswap();
test_index();
test_bit_iter();
+ test_bit_iter_empty();
}
diff --git a/test/unit/bit.result b/test/unit/bit.result
index 1817985fef..37d5f60b3f 100644
--- a/test/unit/bit.result
+++ b/test/unit/bit.result
@@ -889,4 +889,6 @@ bit_index_u64(18446744073709551615, *, -1) => 0 1 2 3 4 5 6 7 8 9 10 11 12 13 14
Set: 3, 9, 11, 16, 17, 18, 22, 24, 25, 27, 29, 64, 65, 68, 69, 72, 73, 76, 77,
Clear: 0, 1, 2, 4, 5, 6, 7, 8, 10, 12, 13, 14, 15, 19, 20, 21, 23, 26, 28, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47, 48, 49, 50, 51, 52, 53, 54, 55, 56, 57, 58, 59, 60, 61, 62, 63, 66, 67, 70, 71, 74, 75, 78, 79,
*** test_bit_iter: done ***
+ *** test_bit_iter_empty ***
+ *** test_bit_iter_empty: done ***
\ No newline at end of file
--
GitLab