[libvirt] [PATCH] hash: fix memory leak and regression
Laine Stump
laine at laine.org
Fri Apr 29 20:21:03 UTC 2011
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.
More information about the libvir-list
mailing list