Skip to content

Commit

Permalink
Fix issue with out-of-bound iterators in merge_insertion_sort
Browse files Browse the repository at this point in the history
This is directly linked to #182: the previous resolution made the tests
pass but was incomplete. The new resolution tries to generally simplify
the related bits of the algorithm and should cover the possible
out-of-bounds issues better.
  • Loading branch information
Morwenn committed Mar 5, 2021
1 parent 735fe51 commit 1160154
Showing 1 changed file with 6 additions and 10 deletions.
16 changes: 6 additions & 10 deletions include/cpp-sort/detail/merge_insertion_sort.h
Original file line number Diff line number Diff line change
Expand Up @@ -340,7 +340,7 @@ namespace detail
////////////////////////////////////////////////////////////
// Binary insertion into the main chain

auto current_it = first + 2;
auto current_it = first;
auto current_pend = pend.begin();

for (int k = 0 ; ; ++k) {
Expand All @@ -358,9 +358,8 @@ namespace detail
auto it = current_it + dist * 2;
auto pe = current_pend + dist;

do {
while (true) {
--pe;
it -= 2;

auto insertion_point = detail::upper_bound(
chain.begin(), *pe, proj(*it),
Expand All @@ -370,7 +369,10 @@ namespace detail
utility::identity{}
);
chain.insert(insertion_point, it);
} while (pe != current_pend);

if (pe == current_pend) break;
it -= 2;
}

current_it += dist * 2;
current_pend += dist;
Expand All @@ -379,12 +381,6 @@ namespace detail
// If there are pend elements left, insert them into the main
// chain, the order of insertion does not matter so forward
// traversal is ok
//
// current_it is guaranteed to be at least first + 2, so this is
// safe - incrementing it at the end of the loop might cause the
// iterator to go out of bounds, so we decrement it here so that
// we can increment it at the beginning of the loop
current_it -= 2;
while (current_pend != pend.end()) {
current_it += 2;
auto insertion_point = detail::upper_bound(
Expand Down

0 comments on commit 1160154

Please sign in to comment.