From 7f3288be65c532f219389d8258b4940bb1e9cc5c Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@redhat.com>
Date: Tue, 21 Jan 2014 19:41:42 -0500
Subject: [PATCH 41/60] libxkutil:pool_parsing: Coverity cleanups

A new version of Coverity found a number of issues:

get_pool_from_xml(): FORWARD_NULL
parse_disk_pool(): RESOURCE_LEAK
  - If parse_disk_pool() returned name == NULL, then the strdup() of
    name into pool->id would dereference the NULL pointer leading to
    a segfault.

    Furthermore parse_disk_pool() had a few issues.  First the 'type_str'
    could be NULL, so that needs to be checked.  Second, 'type_str' was
    never free()'d (the get_attr_value returns a strdup()'d value).

    Realizing all that resulted in a few extra changes to not strdup()
    a value that we strdup()'d

    Eventually get_pool_from_xml() will return to get_disk_pool() which
    returns to diskpool_set_capacity() or _new_volume_template() which
    both error out when return value != 0 (although, I did change the
    latter to be more explicit and match the former).

    Finally, even though the parsing of the element will only ever have
    one "name" value - Coverity doesn't know that, so as a benign fix be
    sure to not overwrite 'name' if "name" isn't already set.

Signed-off-by: John Ferlan <jferlan@redhat.com>
---
 libxkutil/pool_parsing.c              | 39 ++++++++++++++++++-----------------
 src/Virt_SettingsDefineCapabilities.c |  2 +-
 2 files changed, 21 insertions(+), 20 deletions(-)

diff --git a/libxkutil/pool_parsing.c b/libxkutil/pool_parsing.c
index 922ff32..80bd4ca 100644
--- a/libxkutil/pool_parsing.c
+++ b/libxkutil/pool_parsing.c
@@ -194,15 +194,17 @@ char *get_disk_pool_type(uint16_t type)

 }

-static const char *parse_disk_pool(xmlNodeSet *nsv, struct disk_pool *pool)
+static char *parse_disk_pool(xmlNodeSet *nsv, struct disk_pool *pool)
 {
         xmlNode **nodes = nsv->nodeTab;
         xmlNode *child;
-        const char *type_str = NULL;
-        const char *name = NULL;
+        char *type_str = NULL;
+        char *name = NULL;
         int type = 0;

         type_str = get_attr_value(nodes[0], "type");
+        if (type_str == NULL)
+            return NULL;

         if (STREQC(type_str, "dir"))
                 type = DISK_POOL_DIR;
@@ -220,12 +222,15 @@ static const char *parse_disk_pool(xmlNodeSet *nsv, struct disk_pool *pool)
                 type = DISK_POOL_SCSI;
         else
                 type = DISK_POOL_UNKNOWN;
+        free(type_str);

         pool->pool_type = type;
-
+
         for (child = nodes[0]->children; child != NULL; child = child->next) {
-                if (XSTREQ(child->name, "name")) {
+                if (XSTREQ(child->name, "name") && name == NULL) {
                         name = get_node_content(child);
+                        if (name == NULL)
+                            return NULL;
                 } else if (XSTREQ(child->name, "target"))
                         parse_disk_target(child, pool);
                 else if (XSTREQ(child->name, "source"))
@@ -238,14 +243,18 @@ static const char *parse_disk_pool(xmlNodeSet *nsv, struct disk_pool *pool)
 int get_pool_from_xml(const char *xml, struct virt_pool *pool, int type)
 {
         int len;
-        int ret = 0;
+        int ret = 1;
         xmlDoc *xmldoc;
         xmlXPathContext *xpathctx;
         xmlXPathObject *xpathobj;
         const xmlChar *xpathstr = (xmlChar *)"/pool";
-        const char *name;

         CU_DEBUG("Pool XML : %s", xml);
+
+        /* FIXME: Add support for parsing network pools */
+        if (type == CIM_RES_TYPE_NET)
+                return 0;
+
	len = strlen(xml) + 1;

         if ((xmldoc = xmlParseMemory(xml, len)) == NULL)
@@ -257,22 +266,14 @@ int get_pool_from_xml(const char *xml, struct virt_pool *pool, int type)
         if ((xpathobj = xmlXPathEvalExpression(xpathstr, xpathctx)) == NULL)
                 goto err3;

-        /* FIXME: Add support for parsing network pools */
-        if (type == CIM_RES_TYPE_NET) {
-                ret = 0;
-                goto err1;
-        }
-
         memset(pool, 0, sizeof(*pool));

-        pool->type = CIM_RES_TYPE_DISK;
-        name = parse_disk_pool(xpathobj->nodesetval,
-                               &(pool)->pool_info.disk);
-        if (name == NULL)
+        pool->type = CIM_RES_TYPE_DISK;
+        pool->id = parse_disk_pool(xpathobj->nodesetval,
+                                   &(pool)->pool_info.disk);
+        if (pool->id != NULL)
                 ret = 0;

-        pool->id = strdup(name);
-
         xmlXPathFreeObject(xpathobj);
  err3:
         xmlXPathFreeContext(xpathctx);
diff --git a/src/Virt_SettingsDefineCapabilities.c b/src/Virt_SettingsDefineCapabilities.c
index fe16e3f..756e46b 100644
--- a/src/Virt_SettingsDefineCapabilities.c
+++ b/src/Virt_SettingsDefineCapabilities.c
@@ -1376,7 +1376,7 @@ static CMPIStatus _new_volume_template(const CMPIObjectPath *ref,
         }

         ret = get_disk_pool(poolptr, &pool);
-        if (ret == 1) {
+        if (ret != 0) {
                 virt_set_status(_BROKER, &s,
                                 CMPI_RC_ERR_FAILED,
                                 virStoragePoolGetConnect(poolptr),
--
2.1.0