From 13ded99f3ea9eab66ca6d36147981cf415809caf Mon Sep 17 00:00:00 2001 From: Nathan Bronson Date: Tue, 21 Mar 2017 14:28:38 -0700 Subject: [PATCH] fix sorted_vector_{set,map} insert with bad hint Summary: sorted_vector_{set,map} can silently drop insert if the hinted insertion position is too small. This diff fixes it. Reviewed By: yfeldblum Differential Revision: D4747319 fbshipit-source-id: 31e399d07a0b77b700edf034dc723cb997dc8e16 --- folly/sorted_vector_types.h | 16 +++++++--------- folly/test/sorted_vector_test.cpp | 15 +++++++++++++++ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/folly/sorted_vector_types.h b/folly/sorted_vector_types.h index fe586a86..41d26fc0 100644 --- a/folly/sorted_vector_types.h +++ b/folly/sorted_vector_types.h @@ -122,22 +122,20 @@ namespace detail { { const typename OurContainer::value_compare& cmp(sorted.value_comp()); if (hint == cont.end() || cmp(value, *hint)) { - if (hint == cont.begin()) { - po.increase_capacity(cont, cont.begin()); - return cont.insert(cont.begin(), std::move(value)); - } - if (cmp(*(hint - 1), value)) { + if (hint == cont.begin() || cmp(*(hint - 1), value)) { hint = po.increase_capacity(cont, hint); return cont.insert(hint, std::move(value)); + } else { + return sorted.insert(std::move(value)).first; } - return sorted.insert(std::move(value)).first; } if (cmp(*hint, value)) { if (hint + 1 == cont.end() || cmp(value, *(hint + 1))) { - typename OurContainer::iterator it = - po.increase_capacity(cont, hint + 1); - return cont.insert(it, std::move(value)); + hint = po.increase_capacity(cont, hint + 1); + return cont.insert(hint, std::move(value)); + } else { + return sorted.insert(std::move(value)).first; } } diff --git a/folly/test/sorted_vector_test.cpp b/folly/test/sorted_vector_test.cpp index 9207fa03..257b5dff 100644 --- a/folly/test/sorted_vector_test.cpp +++ b/folly/test/sorted_vector_test.cpp @@ -144,6 +144,21 @@ TEST(SortedVectorTypes, SimpleSetTest) { EXPECT_TRUE(cpy2 == cpy); } +TEST(SortedVectorTypes, BadHints) { + for (int toInsert = -1; toInsert <= 7; ++toInsert) { + for (int hintPos = 0; hintPos <= 4; ++hintPos) { + sorted_vector_set s; + for (int i = 0; i <= 3; ++i) { + s.insert(i * 2); + } + s.insert(s.begin() + hintPos, toInsert); + size_t expectedSize = (toInsert % 2) == 0 ? 4 : 5; + EXPECT_EQ(s.size(), expectedSize); + check_invariant(s); + } + } +} + TEST(SortedVectorTypes, SimpleMapTest) { sorted_vector_map m; for (int i = 0; i < 1000; ++i) { -- 2.34.1