From b0bcffda3bafe4356a807f8cdd7f5e1df3addf42 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Thu, 29 May 2008 15:28:28 +0000
Subject: [PATCH] Misc OOM / memory leak fixes

---
 ChangeLog          |  8 ++++++++
 src/capabilities.c | 15 ++++++++++++++-
 src/qemu_conf.c    | 10 +++++++++-
 src/qparams.c      | 27 +++++++++++++++++++++++----
 4 files changed, 54 insertions(+), 6 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index ed7cc0acc9..2b93c43461 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+Thu May 29 11:23:00 EST 2008 Daniel P. Berrange <berrange@redhat.com>
+
+	Misc memory handling / cleanup fixes
+	* src/capabilities.c: Avoiding deferencing NULL pointer in
+	cleanup code
+	* src/qemu_conf.c: Free sound structs on cleanup
+	* src/qparams.c: raise a libvirt error upon OOM
+
 Thu May 29 11:12:00 EST 2008 Daniel P. Berrange <berrange@redhat.com>
 
 	* tests/testutils.c, tests/testutils.h: Add generic main()
diff --git a/src/capabilities.c b/src/capabilities.c
index cf95751ea9..86ee4d3a03 100644
--- a/src/capabilities.c
+++ b/src/capabilities.c
@@ -44,7 +44,7 @@ virCapabilitiesNew(const char *arch,
     virCapsPtr caps;
 
     if (VIR_ALLOC(caps) < 0)
-        goto no_memory;
+        return NULL;
 
     if ((caps->host.arch = strdup(arch)) == NULL)
         goto no_memory;
@@ -61,6 +61,9 @@ virCapabilitiesNew(const char *arch,
 static void
 virCapabilitiesFreeHostNUMACell(virCapsHostNUMACellPtr cell)
 {
+    if (cell == NULL)
+        return;
+
     VIR_FREE(cell->cpus);
     VIR_FREE(cell);
 }
@@ -69,6 +72,9 @@ static void
 virCapabilitiesFreeGuestDomain(virCapsGuestDomainPtr dom)
 {
     int i;
+    if (dom == NULL)
+        return;
+
     VIR_FREE(dom->info.emulator);
     VIR_FREE(dom->info.loader);
     for (i = 0 ; i < dom->info.nmachines ; i++)
@@ -82,6 +88,8 @@ virCapabilitiesFreeGuestDomain(virCapsGuestDomainPtr dom)
 static void
 virCapabilitiesFreeGuestFeature(virCapsGuestFeaturePtr feature)
 {
+    if (feature == NULL)
+        return;
     VIR_FREE(feature->name);
     VIR_FREE(feature);
 }
@@ -90,6 +98,9 @@ static void
 virCapabilitiesFreeGuest(virCapsGuestPtr guest)
 {
     int i;
+    if (guest == NULL)
+        return;
+
     VIR_FREE(guest->ostype);
 
     VIR_FREE(guest->arch.name);
@@ -120,6 +131,8 @@ virCapabilitiesFreeGuest(virCapsGuestPtr guest)
 void
 virCapabilitiesFree(virCapsPtr caps) {
     int i;
+    if (caps == NULL)
+        return;
 
     for (i = 0 ; i < caps->nguests ; i++)
         virCapabilitiesFreeGuest(caps->guests[i]);
diff --git a/src/qemu_conf.c b/src/qemu_conf.c
index cd5dc0d50b..17ef518368 100644
--- a/src/qemu_conf.c
+++ b/src/qemu_conf.c
@@ -214,6 +214,7 @@ void qemudFreeVMDef(struct qemud_vm_def *def) {
     struct qemud_vm_input_def *input = def->inputs;
     struct qemud_vm_chr_def *serial = def->serials;
     struct qemud_vm_chr_def *parallel = def->parallels;
+    struct qemud_vm_sound_def *sound = def->sounds;
 
     while (disk) {
         struct qemud_vm_disk_def *prev = disk;
@@ -240,6 +241,11 @@ void qemudFreeVMDef(struct qemud_vm_def *def) {
         parallel = parallel->next;
         free(prev);
     }
+    while (sound) {
+        struct qemud_vm_sound_def *prev = sound;
+        sound = sound->next;
+        free(prev);
+    }
     xmlFree(def->keymap);
     free(def);
 }
@@ -2186,8 +2192,10 @@ static struct qemud_vm_def *qemudParseXML(virConnectPtr conn,
                 }
                 check = check->next;
             }
-            if (collision)
+            if (collision) {
+                free(sound);
                 continue;
+            }
 
             def->nsounds++;
             sound->next = NULL;
diff --git a/src/qparams.c b/src/qparams.c
index b5514a53df..f068cb4212 100644
--- a/src/qparams.c
+++ b/src/qparams.c
@@ -30,6 +30,14 @@
 #include "memory.h"
 #include "qparams.h"
 
+static void
+qparam_report_oom(void)
+{
+    const char *virerr = __virErrorMsg(VIR_ERR_NO_MEMORY, NULL);
+    __virRaiseError(NULL, NULL, NULL, VIR_FROM_NONE, VIR_ERR_NO_MEMORY, VIR_ERR_ERROR,
+                    virerr, NULL, NULL, -1, -1, virerr, NULL);
+}
+
 struct qparam_set *
 new_qparam_set (int init_alloc, ...)
 {
@@ -39,12 +47,15 @@ new_qparam_set (int init_alloc, ...)
 
     if (init_alloc <= 0) init_alloc = 1;
 
-    if (VIR_ALLOC(ps) < 0)
+    if (VIR_ALLOC(ps) < 0) {
+        qparam_report_oom();
         return NULL;
+    }
     ps->n = 0;
     ps->alloc = init_alloc;
     if (VIR_ALLOC_N(ps->p, ps->alloc) < 0) {
         VIR_FREE (ps);
+        qparam_report_oom();
         return NULL;
     }
 
@@ -88,7 +99,7 @@ grow_qparam_set (struct qparam_set *ps)
 {
     if (ps->n >= ps->alloc) {
         if (VIR_REALLOC_N(ps->p, ps->alloc * 2) < 0) {
-            perror ("realloc");
+            qparam_report_oom();
             return -1;
         }
         ps->alloc *= 2;
@@ -104,12 +115,15 @@ append_qparam (struct qparam_set *ps,
     char *pname, *pvalue;
 
     pname = strdup (name);
-    if (!pname)
+    if (!pname) {
+        qparam_report_oom();
         return -1;
+    }
 
     pvalue = strdup (value);
     if (!pvalue) {
         VIR_FREE (pname);
+        qparam_report_oom();
         return -1;
     }
 
@@ -143,6 +157,7 @@ qparam_get_query (const struct qparam_set *ps)
     }
 
     if (virBufferError(&buf)) {
+        qparam_report_oom();
         return NULL;
     }
 
@@ -169,7 +184,10 @@ qparam_query_parse (const char *query)
     const char *end, *eq;
 
     ps = new_qparam_set (0, NULL);
-    if (!ps) return NULL;
+    if (!ps) {
+        qparam_report_oom();
+        return NULL;
+    }
 
     if (!query || query[0] == '\0') return ps;
 
@@ -240,6 +258,7 @@ qparam_query_parse (const char *query)
     return ps;
 
  out_of_memory:
+    qparam_report_oom();
     free_qparam_set (ps);
     return NULL;
 }