Test the binary search more aggressively.

https://boringssl-review.googlesource.com/c/boringssl/+/32115/ wasn't
worth it, but we may as well keep the test.  Also add a comment about
the asymptotics in case it ever comes up.

Change-Id: Ic4773106f1003adc56b4ce36520a18d3ac2d6f13
Reviewed-on: https://boringssl-review.googlesource.com/32284
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2018-09-23 16:48:24 -05:00 committed by CQ bot account: commit-bot@chromium.org
parent fac6fb99da
commit ce00828c89
2 changed files with 47 additions and 1 deletions

View File

@ -287,7 +287,10 @@ int sk_find(const _STACK *sk, size_t *out_index, const void *p,
return 0; return 0;
} }
size_t idx = ((void **)r) - sk->data; size_t idx = ((void **)r) - sk->data;
// This function always returns the first result. // This function always returns the first result. Note this logic is, in the
// worst case, O(N) rather than O(log(N)). If this ever becomes a problem,
// restore https://boringssl-review.googlesource.com/c/boringssl/+/32115/
// which integrates the preference into the binary search.
while (idx > 0) { while (idx > 0) {
const void *elem = sk->data[idx - 1]; const void *elem = sk->data[idx - 1];
if (call_cmp_func(sk->comp, &p, &elem) != 0) { if (call_cmp_func(sk->comp, &p, &elem) != 0) {

View File

@ -349,3 +349,46 @@ TEST(StackTest, FindFirst) {
ASSERT_TRUE(sk_TEST_INT_find(sk.get(), &index, two)); ASSERT_TRUE(sk_TEST_INT_find(sk.get(), &index, two));
EXPECT_EQ(0u, index); EXPECT_EQ(0u, index);
} }
// Exhaustively test the binary search.
TEST(StackTest, BinarySearch) {
static const size_t kCount = 100;
for (size_t i = 0; i < kCount; i++) {
SCOPED_TRACE(i);
for (size_t j = i; j <= kCount; j++) {
SCOPED_TRACE(j);
// Make a stack where [0, i) are below, [i, j) match, and [j, kCount) are
// above.
bssl::UniquePtr<STACK_OF(TEST_INT)> sk(sk_TEST_INT_new(compare));
ASSERT_TRUE(sk);
for (size_t k = 0; k < i; k++) {
auto value = TEST_INT_new(-1);
ASSERT_TRUE(value);
ASSERT_TRUE(bssl::PushToStack(sk.get(), std::move(value)));
}
for (size_t k = i; k < j; k++) {
auto value = TEST_INT_new(0);
ASSERT_TRUE(value);
ASSERT_TRUE(bssl::PushToStack(sk.get(), std::move(value)));
}
for (size_t k = j; k < kCount; k++) {
auto value = TEST_INT_new(1);
ASSERT_TRUE(value);
ASSERT_TRUE(bssl::PushToStack(sk.get(), std::move(value)));
}
sk_TEST_INT_sort(sk.get());
auto key = TEST_INT_new(0);
ASSERT_TRUE(key);
size_t idx;
int found = sk_TEST_INT_find(sk.get(), &idx, key.get());
if (i == j) {
EXPECT_FALSE(found);
} else {
ASSERT_TRUE(found);
EXPECT_EQ(i, idx);
}
}
}
}