Commit b4ae0a53 authored by Casey Duncan's avatar Casey Duncan

Fix bug indexing BTreeItems with negative values. Previous code assumed that...

Fix bug indexing BTreeItems with negative values. Previous code assumed that idexes passed to BTreeItem_item would be negative, in practice this was not the case, and an index of -1 actually passed len(items)-1 as the index argument. Consequently, it was possible to access each item using two negative indexes (i.e., -1 and -len(items)-1). This made each item appear twice in a reverse iteration.

Code in BTreeItems_seek attempted to match the sign of the pseudoindex kept in the BTreeItems obj and the incoming index. Since this incoming index was never legitimately negative as it assumed, actually negatives passed in due to "overshooting" the first item would repeat the items again from the end. Removal of this code corrects the problem.

Unittests which provoke the error in the original code have also been added.
parent 80c7c3b6
...@@ -143,20 +143,6 @@ BTreeItems_seek(BTreeItems *self, int i) ...@@ -143,20 +143,6 @@ BTreeItems_seek(BTreeItems *self, int i)
currentbucket = self->currentbucket; currentbucket = self->currentbucket;
if (currentbucket == NULL) goto no_match; if (currentbucket == NULL) goto no_match;
/* Make sure that the index and pseudoindex have the same sign. */
if (pseudoindex < 0 && i >= 0) {
/* Position to the start of the sequence. */
currentbucket = self->firstbucket;
currentoffset = self->first;
pseudoindex = 0;
}
else if (pseudoindex >= 0 && i < 0) {
/* Position to the end of the sequence. */
currentbucket = self->lastbucket;
currentoffset = self->last;
pseudoindex = -1;
}
delta = i - pseudoindex; delta = i - pseudoindex;
while (delta > 0) { /* move right */ while (delta > 0) { /* move right */
int max; int max;
......
...@@ -187,6 +187,7 @@ class MappingBase(Base): ...@@ -187,6 +187,7 @@ class MappingBase(Base):
v = self.t.values() v = self.t.values()
for i in range(100): for i in range(100):
self.assertEqual(v[i], i*i) self.assertEqual(v[i], i*i)
self.assertRaises(IndexError, lambda: v[i+1])
i = 0 i = 0
for value in self.t.itervalues(): for value in self.t.itervalues():
self.assertEqual(value, i*i) self.assertEqual(value, i*i)
...@@ -204,6 +205,16 @@ class MappingBase(Base): ...@@ -204,6 +205,16 @@ class MappingBase(Base):
lst = list(self.t.values(max=99-x, min=0+x)) lst = list(self.t.values(max=99-x, min=0+x))
lst.sort() lst.sort()
self.assertEqual(lst,range(0+x,99-x+1)) self.assertEqual(lst,range(0+x,99-x+1))
def testValuesNegativeIndex(self):
L = [-3, 6, -11, 4]
for i in L:
self.t[i] = i
L.sort()
vals = self.t.values()
for i in range(-1, -4, -1):
self.assertEqual(vals[i], L[i])
self.assertRaises(IndexError, lambda: vals[-5])
def testKeysWorks(self): def testKeysWorks(self):
for x in range(100): for x in range(100):
...@@ -213,6 +224,7 @@ class MappingBase(Base): ...@@ -213,6 +224,7 @@ class MappingBase(Base):
for x in v: for x in v:
self.assertEqual(x,i) self.assertEqual(x,i)
i = i + 1 i = i + 1
self.assertRaises(IndexError, lambda: v[i])
for x in range(40): for x in range(40):
lst = self.t.keys(0+x,99-x) lst = self.t.keys(0+x,99-x)
...@@ -222,6 +234,16 @@ class MappingBase(Base): ...@@ -222,6 +234,16 @@ class MappingBase(Base):
self.assertEqual(list(lst), range(0+x, 99-x+1)) self.assertEqual(list(lst), range(0+x, 99-x+1))
self.assertEqual(len(v), 100) self.assertEqual(len(v), 100)
def testKeysNegativeIndex(self):
L = [-3, 6, -11, 4]
for i in L:
self.t[i] = i
L.sort()
keys = self.t.keys()
for i in range(-1, -4, -1):
self.assertEqual(keys[i], L[i])
self.assertRaises(IndexError, lambda: keys[-5])
def testItemsWorks(self): def testItemsWorks(self):
for x in range(100): for x in range(100):
...@@ -232,6 +254,7 @@ class MappingBase(Base): ...@@ -232,6 +254,7 @@ class MappingBase(Base):
self.assertEqual(x[0], i) self.assertEqual(x[0], i)
self.assertEqual(x[1], 2*i) self.assertEqual(x[1], 2*i)
i += 1 i += 1
self.assertRaises(IndexError, lambda: v[i+1])
i = 0 i = 0
for x in self.t.iteritems(): for x in self.t.iteritems():
...@@ -243,7 +266,16 @@ class MappingBase(Base): ...@@ -243,7 +266,16 @@ class MappingBase(Base):
items = list(self.t.iteritems(min=12, max=20)) items = list(self.t.iteritems(min=12, max=20))
self.assertEqual(items, zip(range(12, 21), range(24, 43, 2))) self.assertEqual(items, zip(range(12, 21), range(24, 43, 2)))
def testItemsNegativeIndex(self):
L = [-3, 6, -11, 4]
for i in L:
self.t[i] = i
L.sort()
items = self.t.items()
for i in range(-1, -4, -1):
self.assertEqual(items[i], (L[i], L[i]))
self.assertRaises(IndexError, lambda: items[-5])
def testDeleteInvalidKeyRaisesKeyError(self): def testDeleteInvalidKeyRaisesKeyError(self):
self.assertRaises(KeyError, self._deletefail) self.assertRaises(KeyError, self._deletefail)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment