On 04/29/2011 03:53 PM, Eric Blake wrote:
Commit 1671d1d introduced a memory leak in virHashFree, and
wholesale table corruption in virHashRemoveSet (elements not
requested to be freed are lost).
* src/util/hash.c (virHashFree): Free bucket array.
(virHashRemoveSet): Don't lose elements.
* tests/hashtest.c (testHashCheckForEachCount): New method.
(testHashCheckCount): Expose the bug.
---
src/util/hash.c | 6 +++---
tests/hashtest.c | 15 +++++++++++++++
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/src/util/hash.c b/src/util/hash.c
index a9b09b0..5366dd6 100644
--- a/src/util/hash.c
+++ b/src/util/hash.c
@@ -269,6 +269,7 @@ virHashFree(virHashTablePtr table)
}
}
+ VIR_FREE(table->table);
VIR_FREE(table);
}
@@ -532,13 +533,12 @@ int virHashForEach(virHashTablePtr table, virHashIterator iter, void *data)
* virHashRemoveSet
* @table: the hash table to process
* @iter: callback to identify elements for removal
- * @f: callback to free memory from element payload
* @data: opaque data to pass to the iterator
*
* Iterates over all elements in the hash table, invoking the 'iter'
* callback. If the callback returns a non-zero value, the element
* will be removed from the hash table& its payload passed to the
- * callback 'f' for de-allocation.
+ * data freer callback registered at creation.
*
* Returns number of items removed on success, -1 on failure
*/
@@ -562,7 +562,7 @@ int virHashRemoveSet(virHashTablePtr table,
while (*nextptr) {
virHashEntryPtr entry = *nextptr;
if (!iter(entry->payload, entry->name, data)) {
- *nextptr = entry->next;
+ nextptr =&entry->next;
} else {
count++;
if (table->dataFree)
diff --git a/tests/hashtest.c b/tests/hashtest.c
index 525ae06..f02b3a9 100644
--- a/tests/hashtest.c
+++ b/tests/hashtest.c
@@ -61,16 +61,31 @@ testHashInit(int size)
return hash;
}
+static void
+testHashCheckForEachCount(void *payload ATTRIBUTE_UNUSED,
+ const void *name ATTRIBUTE_UNUSED,
+ void *data ATTRIBUTE_UNUSED)
+{
+}
static int
testHashCheckCount(virHashTablePtr hash, int count)
{
+ int iter_count = 0;
+
if (virHashSize(hash) != count) {
testError("\nhash contains %d instead of %d elements\n",
virHashSize(hash), count);
return -1;
}
+ iter_count = virHashForEach(hash, testHashCheckForEachCount, NULL);
+ if (count != iter_count) {
+ testError("\nhash claims to have %d elements but iteration finds %d\n",
+ count, iter_count);
+ return -1;
+ }
+
return 0;
}
ACK.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list