Skip to content

Commit 862fa4d

Browse files
committed
Fixes a bug when removing items from a JKDictionary. Since JKDictionary is implemented using a hash table that uses linear probing, the removal function needs to "re-add" items that follow the removed item so that linear probe hash collisions are not "lost". Closes johnezang#17
1 parent 2e6ccf4 commit 862fa4d

File tree

1 file changed

+21
-2
lines changed

1 file changed

+21
-2
lines changed

JSONKit.m

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -937,11 +937,30 @@ static NSUInteger _JKDictionaryCapacity(JKDictionary *dictionary) {
937937
}
938938

939939
static void _JKDictionaryRemoveObjectWithEntry(JKDictionary *dictionary, JKHashTableEntry *entry) {
940-
NSCParameterAssert((dictionary != NULL) && (entry != NULL) && (entry->key != NULL) && (entry->object != NULL) && (dictionary->count > 0UL));
940+
NSCParameterAssert((dictionary != NULL) && (entry != NULL) && (entry->key != NULL) && (entry->object != NULL) && (dictionary->count > 0UL) && (dictionary->count <= dictionary->capacity));
941941
CFRelease(entry->key); entry->key = NULL;
942942
CFRelease(entry->object); entry->object = NULL;
943943
entry->keyHash = 0UL;
944944
dictionary->count--;
945+
// In order for certain invariants that are used to speed up the search for a particular key, we need to "re-add" all the entries in the hash table following this entry until we hit a NULL entry.
946+
NSUInteger removeIdx = entry - dictionary->entry, idx = 0UL;
947+
NSCParameterAssert((removeIdx < dictionary->capacity));
948+
for(idx = 0UL; idx < dictionary->capacity; idx++) {
949+
NSUInteger entryIdx = (removeIdx + idx + 1UL) % dictionary->capacity;
950+
JKHashTableEntry *atEntry = &dictionary->entry[entryIdx];
951+
if(atEntry->key == NULL) { break; }
952+
NSUInteger keyHash = atEntry->keyHash;
953+
id key = atEntry->key, object = atEntry->object;
954+
NSCParameterAssert(object != NULL);
955+
atEntry->keyHash = 0UL;
956+
atEntry->key = NULL;
957+
atEntry->object = NULL;
958+
NSUInteger addKeyEntry = keyHash % dictionary->capacity, addIdx = 0UL;
959+
for(addIdx = 0UL; addIdx < dictionary->capacity; addIdx++) {
960+
JKHashTableEntry *atAddEntry = &dictionary->entry[((addKeyEntry + addIdx) % dictionary->capacity)];
961+
if(JK_EXPECT_T(atAddEntry->key == NULL)) { NSCParameterAssert((atAddEntry->keyHash == 0UL) && (atAddEntry->object == NULL)); atAddEntry->key = key; atAddEntry->object = object; atAddEntry->keyHash = keyHash; break; }
962+
}
963+
}
945964
}
946965

947966
static void _JKDictionaryAddObject(JKDictionary *dictionary, NSUInteger keyHash, id key, id object) {
@@ -951,7 +970,7 @@ static void _JKDictionaryAddObject(JKDictionary *dictionary, NSUInteger keyHash,
951970
NSUInteger entryIdx = (keyEntry + idx) % dictionary->capacity;
952971
JKHashTableEntry *atEntry = &dictionary->entry[entryIdx];
953972
if(JK_EXPECT_F(atEntry->keyHash == keyHash) && JK_EXPECT_T(atEntry->key != NULL) && (JK_EXPECT_F(key == atEntry->key) || JK_EXPECT_F(CFEqual(atEntry->key, key)))) { _JKDictionaryRemoveObjectWithEntry(dictionary, atEntry); }
954-
if(JK_EXPECT_T(atEntry->key == NULL)) { atEntry->key = key; atEntry->object = object; atEntry->keyHash = keyHash; dictionary->count++; return; }
973+
if(JK_EXPECT_T(atEntry->key == NULL)) { NSCParameterAssert((atEntry->keyHash == 0UL) && (atEntry->object == NULL)); atEntry->key = key; atEntry->object = object; atEntry->keyHash = keyHash; dictionary->count++; return; }
955974
}
956975

957976
// We should never get here. If we do, we -release the key / object because it's our responsibility.

0 commit comments

Comments
 (0)