Fixed a nasty bug in HashDictionary that could cause heap corruption after removing a value. Added a test case to catch that bug. A few tweaks to Hash.
1.1 --- a/include/Dictionary.h Sun Sep 20 21:25:47 2009 -0700
1.2 +++ b/include/Dictionary.h Thu Sep 24 10:28:50 2009 -0700
1.3 @@ -24,6 +24,7 @@
1.4 explicit Key (const Blob &b) :Blob(b), hash(computeHash(b)) { }
1.5 Key (const Blob &b, HashCode h) :Blob(b), hash(h) { }
1.6 Key (const char *str) :Blob(str), hash(computeHash(*this)) { }
1.7 + Key (const void *bytes, uint32_t len) :Blob(bytes,len), hash(computeHash(*this)) { }
1.8 Key (const void *b, uint32_t len, HashCode h) :Blob(b,len), hash(h) { }
1.9
1.10 static HashCode computeHash (Blob);
2.1 --- a/src/Dictionary.cpp Sun Sep 20 21:25:47 2009 -0700
2.2 +++ b/src/Dictionary.cpp Thu Sep 24 10:28:50 2009 -0700
2.3 @@ -186,9 +186,9 @@
2.4 }
2.5
2.6 bool HashDictionary::remove (Key key) {
2.7 - KeyAndValue *kv = (KeyAndValue*) _hash.get(key);
2.8 + KeyAndValue *kv = (KeyAndValue*) _hash.remove(key);
2.9 if (kv) {
2.10 - free(kv);
2.11 + delete kv;
2.12 return true;
2.13 } else
2.14 return false;
2.15 @@ -196,7 +196,7 @@
2.16
2.17 void HashDictionary::removeAll() {
2.18 for (Hash::Iterator it(&_hash); it; ++it)
2.19 - free(it.value());
2.20 + delete (KeyAndValue*) it.value();
2.21 _hash.removeAll();
2.22 }
2.23
3.1 --- a/src/Hash.cpp Sun Sep 20 21:25:47 2009 -0700
3.2 +++ b/src/Hash.cpp Thu Sep 24 10:28:50 2009 -0700
3.3 @@ -20,7 +20,7 @@
3.4 _value(value)
3.5 { }
3.6
3.7 - Key key() const {return _key;}
3.8 + const Key& key() const {return _key;}
3.9 HashCode hash() const {return _key.hash;}
3.10 Hash::Value value() const {return _key.bytes ?_value :NULL;}
3.11 void setValue (Hash::Value value) {_value = value;}
3.12 @@ -50,8 +50,7 @@
3.13 }
3.14
3.15 Hash::IndexEntry& Hash::_find (Key key) const {
3.16 - Key k = Key(key);
3.17 - IndexEntry newEntry(k);
3.18 + IndexEntry newEntry(key);
3.19 int index = newEntry.hash() % _tableSize;
3.20 IndexEntry *found = &_table[index];
3.21 int probe = 0;
3.22 @@ -78,7 +77,7 @@
3.23 }
3.24
3.25 void Hash::put(Key key, Hash::Value value) {
3.26 - IndexEntry newEntry = IndexEntry(Key(key),value);
3.27 + IndexEntry newEntry = IndexEntry(key,value);
3.28 IndexEntry &found = _findForPut(newEntry);
3.29 if (found.hasValue())
3.30 found.setValue(value);
3.31 @@ -90,15 +89,16 @@
3.32 }
3.33 }
3.34
3.35 - bool Hash::remove(Key key) {
3.36 + Hash::Value Hash::remove(Key key) {
3.37 IndexEntry &entry = _find(key);
3.38 - if (!entry.value())
3.39 - return false;
3.40 + Value formerValue = entry.value();
3.41 + if (!formerValue)
3.42 + return NULL;
3.43 entry.markDeleted();
3.44 _count--;
3.45 if (_count/(float)_tableSize < Dictionary::kMinLoadFactor)
3.46 _resize(_tableSize/2);
3.47 - return true;
3.48 + return formerValue;
3.49 }
3.50
3.51 void Hash::removeAll() {
3.52 @@ -109,7 +109,7 @@
3.53 void Hash::dump() const {
3.54 for (int index=0; index<_tableSize; index++)
3.55 if (_table[index].hasValue()) {
3.56 - Blob key = _table[index].key();
3.57 + const Key &key = _table[index].key();
3.58 printf("%3i: %*s -> %p\n",
3.59 index, (int)key.length,(const char*)key.bytes, _table[index].value());
3.60 }
4.1 --- a/src/Hash.h Sun Sep 20 21:25:47 2009 -0700
4.2 +++ b/src/Hash.h Thu Sep 24 10:28:50 2009 -0700
4.3 @@ -38,8 +38,8 @@
4.4 /** Sets the given value for the given key, replacing any existing value. */
4.5 void put(Key, Value);
4.6
4.7 - /** Removes the value with the given key. */
4.8 - bool remove(Key);
4.9 + /** Removes the value with the given key. Returns the former value, or NULL if not found. */
4.10 + Value remove(Key);
4.11
4.12 /** Removes all values. */
4.13 void removeAll();
5.1 --- a/test/Dictionary_test.cpp Sun Sep 20 21:25:47 2009 -0700
5.2 +++ b/test/Dictionary_test.cpp Thu Sep 24 10:28:50 2009 -0700
5.3 @@ -32,6 +32,26 @@
5.4 return *sDict;
5.5 }
5.6
5.7 +TEST(Dictionary,AddRemove) {
5.8 + HashDictionary dict;
5.9 + EXPECT_EQ(0, dict.count());
5.10 + dict.put("key 1", "value 1");
5.11 + EXPECT_EQ(1, dict.count());
5.12 + dict.put("key 2", "value 2");
5.13 + EXPECT_EQ(2, dict.count());
5.14 + dict.put("key 3", "value 3");
5.15 + EXPECT_TRUE(dict.get("key 1").equals("value 1"));
5.16 + EXPECT_TRUE(dict.get("key 2").equals("value 2"));
5.17 + EXPECT_TRUE(dict.get("key 3").equals("value 3"));
5.18 + EXPECT_TRUE(!dict.get("key 4"));
5.19 +
5.20 + EXPECT_TRUE(dict.remove("key 2"));
5.21 + EXPECT_EQ(2, dict.count());
5.22 + EXPECT_TRUE(dict.get("key 1").equals("value 1"));
5.23 + EXPECT_TRUE(!dict.get("key 2"));
5.24 + EXPECT_TRUE(dict.get("key 3").equals("value 3"));
5.25 +}
5.26 +
5.27 TEST(Dictionary,GetAll) {
5.28 const Dictionary &dict = getDict();
5.29 EXPECT_EQ( sNWords , dict.count() );