Re: [PATCH] hash: fix memory leak and regression

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]