You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
141 lines
5.1 KiB
141 lines
5.1 KiB
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
|
|
|