From bbdce159fad472cc22073bf064037992b187d0b2 Mon Sep 17 00:00:00 2001 From: Giovanni Campagna Date: Tue, 20 Dec 2011 21:30:30 +0100 Subject: [PATCH] Util: fix binary search exit condition The loop can exit with an interval of length one or one of length zero. In the first case it is correct to check which side of the interval to return, in the second case no comparison should be made (since there is only one possible value). In practice, this usually results in one comparison more than needed, but in some cases (when the position was past the end of the array), would call the comparator with undefined. https://bugzilla.gnome.org/show_bug.cgi?id=666614 --- js/misc/util.js | 2 +- tests/unit/insertSorted.js | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/js/misc/util.js b/js/misc/util.js index 36640580b..8673f8329 100644 --- a/js/misc/util.js +++ b/js/misc/util.js @@ -265,7 +265,7 @@ function lowerBound(array, val, cmp) { max = mid; } - return (cmp(array[min], val) < 0) ? max : min; + return (min == max || cmp(array[min], val) < 0) ? max : min; } // insertSorted: diff --git a/tests/unit/insertSorted.js b/tests/unit/insertSorted.js index e23848a22..610aeed56 100644 --- a/tests/unit/insertSorted.js +++ b/tests/unit/insertSorted.js @@ -64,8 +64,13 @@ let arrayEmpty = []; // inserting in a empty array Util.insertSorted(arrayEmpty, 3, checkedCmp); +// Insert at the end and check that we don't +// access past it +Util.insertSorted(arrayEmpty, 4, checkedCmp); +Util.insertSorted(arrayEmpty, 5, checkedCmp); + // Some more insertions Util.insertSorted(arrayEmpty, 2, checkedCmp); Util.insertSorted(arrayEmpty, 1, checkedCmp); -assertArrayEquals('checkedCmp test', [1, 2, 3], arrayEmpty); +assertArrayEquals('checkedCmp test', [1, 2, 3, 4, 5], arrayEmpty);