On Tue, Jun 19, 2012 at 12:35:40PM -0300, Eduardo Lima (Etrunko) wrote: > From: "Eduardo Lima (Etrunko)" <[email protected]> > > Signed-off-by: Eduardo Lima (Etrunko) <[email protected]> > --- > libxkutil/pool_parsing.c | 27 ++++++------------------- > libxkutil/pool_parsing.h | 4 ++-- > libxkutil/xmlgen.c | 29 > +++++++++++++++++---------- > src/Virt_ResourcePoolConfigurationService.c | 28 +++++++++----------------- > 4 files changed, 35 insertions(+), 53 deletions(-) > > diff --git a/libxkutil/pool_parsing.c b/libxkutil/pool_parsing.c > index e41fc09..bb616d4 100644 > --- a/libxkutil/pool_parsing.c > +++ b/libxkutil/pool_parsing.c > @@ -54,8 +54,6 @@ static void cleanup_net_pool(struct net_pool pool) { > } > > static void cleanup_disk_pool(struct disk_pool pool) { > - uint16_t i; > - > free(pool.path); > free(pool.host); > free(pool.src_dir); > @@ -63,10 +61,7 @@ static void cleanup_disk_pool(struct disk_pool pool) { > free(pool.port_name); > free(pool.node_name); > > - for (i = 0; i < pool.device_paths_ct; i++) > - free(pool.device_paths[i]); > - > - free(pool.device_paths); > + list_free(pool.device_paths); > } > > void cleanup_virt_pool(struct virt_pool **pool) > @@ -130,22 +125,15 @@ static int parse_disk_target(xmlNode *node, struct > disk_pool *pool) > static int parse_disk_source(xmlNode *node, struct disk_pool *pool) > { > xmlNode *child; > - char **dev_paths = NULL; > - unsigned ct = 0; > > for (child = node->children; child != NULL; child = child->next) { > if (XSTREQ(child->name, "device")) { > - char **tmp = NULL; > - > - tmp = realloc(dev_paths, sizeof(char *) * (ct + 1)); > - if (tmp == NULL) { > - CU_DEBUG("Could not alloc space for dev > path"); > - continue; > - } > - dev_paths = tmp; > + if (pool->device_paths == NULL) > + pool->device_paths = list_new(free, > + (list_data_cmp_cb) > strcmp); > > - dev_paths[ct] = get_attr_value(child, "path"); > - ct++; > + list_append(pool->device_paths, > + get_attr_value(child, "path")); > } else if (XSTREQ(child->name, "host")) { > pool->host = get_attr_value(child, "name"); > if (pool->host == NULL) > @@ -161,12 +149,9 @@ static int parse_disk_source(xmlNode *node, struct > disk_pool *pool) > } > } > > - pool->device_paths_ct = ct; > - pool->device_paths = dev_paths; > return 1; > > err: > - free(dev_paths); > return 0; > } > > diff --git a/libxkutil/pool_parsing.h b/libxkutil/pool_parsing.h > index 9f1a386..50fbeac 100644 > --- a/libxkutil/pool_parsing.h > +++ b/libxkutil/pool_parsing.h > @@ -26,6 +26,7 @@ > #include <libvirt/libvirt.h> > > #include "../src/svpc_types.h" > +#include "list_util.h" > > struct net_pool { > char *addr; > @@ -46,8 +47,7 @@ struct disk_pool { > DISK_POOL_LOGICAL, > DISK_POOL_SCSI} pool_type; > char *path; > - char **device_paths; > - uint16_t device_paths_ct; > + list_t *device_paths; > char *host; > char *src_dir; > char *adapter; > diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c > index 2dcd0d2..26e7203 100644 > --- a/libxkutil/xmlgen.c > +++ b/libxkutil/xmlgen.c > @@ -1177,27 +1177,34 @@ static const char *disk_pool_type_to_str(uint16_t > type) > return NULL; > } > > +static bool device_paths_foreach(void *list_data, void *user_data) > +{ > + char *dev_path = (char *) list_data; > + xmlNodePtr src = (xmlNodePtr) user_data; > + xmlNodePtr tmp = NULL; > + > + tmp = xmlNewChild(src, NULL, BAD_CAST "device", BAD_CAST NULL); > + if (tmp == NULL) > + return false; > + > + if (xmlNewProp(tmp, BAD_CAST "path", BAD_CAST dev_path) == NULL) > + return false; > + > + return true; > +} > + > static const char *set_disk_pool_source(xmlNodePtr disk, > struct disk_pool *pool) > { > xmlNodePtr src; > xmlNodePtr tmp; > - uint16_t i; > > src = xmlNewChild(disk, NULL, BAD_CAST "source", NULL); > if (src == NULL) > return XML_ERROR; > > - for (i = 0; i < pool->device_paths_ct; i++) { > - tmp = xmlNewChild(src, NULL, BAD_CAST "device", BAD_CAST > NULL); > - if (tmp == NULL) > - return XML_ERROR; > - > - if (xmlNewProp(tmp, > - BAD_CAST "path", > - BAD_CAST pool->device_paths[i]) == NULL) > - return XML_ERROR; > - } > + if (!list_foreach(pool->device_paths, device_paths_foreach, src)) > + return XML_ERROR; > > if (pool->host != NULL) { > tmp = xmlNewChild(src, NULL, BAD_CAST "host", BAD_CAST NULL); > diff --git a/src/Virt_ResourcePoolConfigurationService.c > b/src/Virt_ResourcePoolConfigurationService.c > index 751d016..bc3e431 100644 > --- a/src/Virt_ResourcePoolConfigurationService.c > +++ b/src/Virt_ResourcePoolConfigurationService.c > @@ -143,7 +143,6 @@ static const char *net_rasd_to_pool(CMPIInstance *inst, > static void init_disk_pool(struct virt_pool *pool) > { > pool->pool_info.disk.device_paths = NULL; > - pool->pool_info.disk.device_paths_ct = 0; > pool->pool_info.disk.path = NULL; > pool->pool_info.disk.host = NULL; > pool->pool_info.disk.src_dir = NULL; > @@ -153,9 +152,8 @@ static void init_disk_pool(struct virt_pool *pool) > pool->pool_info.disk.autostart = 0; > } > > -static char *get_dev_paths(CMPIInstance *inst, > - char ***path_list, > - uint16_t *count) > + > +static char *get_dev_paths(CMPIInstance *inst, list_t **path_list) > { > CMPICount i; > CMPICount ct; > @@ -170,11 +168,8 @@ static char *get_dev_paths(CMPIInstance *inst, > if ((s.rc != CMPI_RC_OK) || (ct <= 0)) > return "Unable to get DevicePaths array count"; > > - *path_list = calloc(ct, sizeof(char *)); > if (*path_list == NULL) > - return "Failed to alloc space for device paths"; > - > - *count = ct; > + *path_list = list_new(free, (list_data_cmp_cb) strcmp); > > for (i = 0; i < ct; i++) { > const char *str = NULL; > @@ -187,7 +182,7 @@ static char *get_dev_paths(CMPIInstance *inst, > if (str == NULL) > return "Unable to get value of DevicePaths element"; > > - *path_list[i] = strdup(str); > + list_append(*path_list, strdup(str)); > } > > return NULL; > @@ -198,10 +193,7 @@ static const char > *disk_fs_or_disk_or_logical_pool(CMPIInstance *inst, > { > const char *msg = NULL; > > - msg = get_dev_paths(inst, > - &pool->pool_info.disk.device_paths, > - &pool->pool_info.disk.device_paths_ct); > - > + msg = get_dev_paths(inst, &pool->pool_info.disk.device_paths); > > /* Specifying a value for DevicePaths isn't mandatory for logical > pool types. */ > @@ -209,7 +201,7 @@ static const char > *disk_fs_or_disk_or_logical_pool(CMPIInstance *inst, > if (msg != NULL) > return msg; > > - if (pool->pool_info.disk.device_paths_ct != 1) { > + if (list_count(pool->pool_info.disk.device_paths) != 1) { > CU_DEBUG("%d only takes one device path", > pool->pool_info.disk.pool_type); > return "Specified pool type only takes one device > path"; > @@ -243,14 +235,12 @@ static const char *disk_iscsi_pool(CMPIInstance *inst, > const char *val = NULL; > const char *msg = NULL; > > - msg = get_dev_paths(inst, > - &pool->pool_info.disk.device_paths, > - &pool->pool_info.disk.device_paths_ct); > - > + msg = get_dev_paths(inst, &pool->pool_info.disk.device_paths); > + > if (msg != NULL) > return msg; > > - if (pool->pool_info.disk.device_paths_ct != 1) > + if (list_count(pool->pool_info.disk.device_paths) != 1) > return "Specified pool type only takes one device path"; > > if (cu_get_str_prop(inst, "Host", &val) != CMPI_RC_OK)
Looks fine but I would rename device_paths_foreach() to indicates what the function does instead of how it is used, something like device_path_dump_xml() or something similar, That doesn't affect the code semantic though ACK, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ [email protected] | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ _______________________________________________ Libvirt-cim mailing list [email protected] https://www.redhat.com/mailman/listinfo/libvirt-cim
