From: "Eduardo Lima (Etrunko)"<eblima(a)br.ibm.com> The list pointer was leaking in the case where the list was empty. Signed-off-by: Eduardo Lima (Etrunko)<eblima(a)br.ibm.com> --- libxkutil/list_util.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/libxkutil/list_util.c b/libxkutil/list_util.c index 84b2ba0..cab0287 100644 --- a/libxkutil/list_util.c +++ b/libxkutil/list_util.c @@ -54,9 +54,12 @@ void list_free(list_t *list) { list_node_t *n, *next; - if (list == NULL || list->head == NULL) + if (list == NULL) return; + if (list->head == NULL) + goto end; + n = list->head; do { @@ -68,6 +71,7 @@ void list_free(list_t *list) n = next; } while (n != list->head); + end: free(list); }

From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com&gt; Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com&gt; --- libxkutil/acl_parsing.c | 71 +++++++++------------------------------- libxkutil/acl_parsing.h | 5 +-- src/Virt_EntriesInFilterList.c | 49 ++++++++++++++------------- src/Virt_FilterEntry.c | 35 +++++++------------- src/Virt_NestedFilterList.c | 3 -- 5 files changed, 56 insertions(+), 107 deletions(-) diff --git a/libxkutil/acl_parsing.c b/libxkutil/acl_parsing.c index 41ab319..8156854 100644 --- a/libxkutil/acl_parsing.c +++ b/libxkutil/acl_parsing.c @@ -126,8 +126,6 @@ void cleanup_rule(struct acl_rule *rule) void cleanup_filter(struct acl_filter *filter) { - int i; - if(filter == NULL) return; @@ -136,12 +134,7 @@ void cleanup_filter(struct acl_filter *filter) free(filter->chain); free(filter->priority); - for (i = 0; i < filter->rule_ct; i++) - cleanup_rule(filter->rules[i]); - - free(filter->rules); - filter->rule_ct = 0; - + list_free(filter->rules); list_free(filter->refs); } @@ -574,53 +567,39 @@ int delete_filter(virConnectPtr conn, struct acl_filter *filter) #endif } -int append_filter_rule(struct acl_filter *filter, struct acl_rule *rule) +static int filter_rule_cmp(void *list_data, void *user_data) { - struct acl_rule **old_rules = NULL; + struct acl_rule * rule = (struct acl_rule *) list_data; + const char * name = (const char *) user_data; - if ((filter == NULL) || (rule == NULL)) - return 0; + return strcmp(rule->name, name); +} - rule->name = make_rule_id(filter->name, filter->rule_ct); - if (rule->name == NULL) +int append_filter_rule(struct acl_filter *filter, struct acl_rule *rule) +{ + if ((filter == NULL) || (rule == NULL)) return 0; - old_rules = filter->rules; + if (filter->rules == NULL) + filter->rules = list_new((list_data_free_cb) cleanup_rule, + filter_rule_cmp); - filter->rules = - malloc((filter->rule_ct + 1) * sizeof(struct acl_rule *)); - - if (filter->rules == NULL) { - CU_DEBUG("Failed to allocate memory for new rule"); - filter->rules = old_rules; + rule->name = make_rule_id(filter->name, list_count(filter->rules)); + if (rule->name == NULL) return 0; - } - - memcpy(filter->rules, - old_rules, - filter->rule_ct * sizeof(struct acl_rule *)); - filter->rules[filter->rule_ct] = rule; - filter->rule_ct++; - - free(old_rules); + list_append(filter->rules, rule); return 1; } - -static int filter_ref_cmp(void *list_data, void *user_data) -{ - return strcmp((const char *)list_data, (const char *) user_data); -} - int append_filter_ref(struct acl_filter *filter, char *name) { if (filter == NULL || name == NULL) return 0; if (filter->refs == NULL) - filter->refs = list_new(free, filter_ref_cmp); + filter->refs = list_new(free, (list_data_cmp_cb) strcmp); if (list_find(filter->refs, name) != NULL) { free(name); @@ -659,24 +638,6 @@ char *make_rule_id(const char *filter, int index) return rule_id; } - -int parse_rule_id(const char *rule_id, char **filter, int *index) -{ - int ret; - - if ((filter == NULL) || (index == NULL)) - return 0; - ret = sscanf(rule_id, "%as[^:]:%u", filter, index); - if (ret != 2) { - free(*filter); - *filter = NULL; - - return 0; - } - - return 1; -} - /* * Local Variables: * mode: C diff --git a/libxkutil/acl_parsing.h b/libxkutil/acl_parsing.h index a0e2f9a..4dca43f 100644 --- a/libxkutil/acl_parsing.h +++ b/libxkutil/acl_parsing.h @@ -152,9 +152,7 @@ struct acl_filter { char *chain; char *priority; - struct acl_rule **rules; - int rule_ct; - + list_t *rules; list_t *refs; }; @@ -171,7 +169,6 @@ int get_filter_by_name(virConnectPtr conn, const char *name, struct acl_filter **filter); char *make_rule_id(const char *filter, int index); -int parse_rule_id(const char *rule_id, char **filter, int *index); int create_filter(virConnectPtr conn, struct acl_filter *filter); int update_filter(virConnectPtr conn, struct acl_filter *filter); diff --git a/src/Virt_EntriesInFilterList.c b/src/Virt_EntriesInFilterList.c index 2c8ac47..ccaa751 100644 --- a/src/Virt_EntriesInFilterList.c +++ b/src/Virt_EntriesInFilterList.c @@ -45,9 +45,10 @@ static CMPIStatus list_to_rule( CMPIStatus s = {CMPI_RC_OK, NULL}; CMPIInstance *instance = NULL; struct acl_filter *filter = NULL; + struct acl_rule *rule = NULL; const char *name = NULL; virConnectPtr conn = NULL; - int i; + list_node_t *head, *node; CU_DEBUG("Reference = %s", REF2STR(reference)); @@ -68,21 +69,29 @@ static CMPIStatus list_to_rule( goto out; } - for (i = 0; i < filter->rule_ct; i++) { - CU_DEBUG("Processing %s", filter->rules[i]->name); + head = node = list_first_node(filter->rules); + if (head == NULL) + goto end; + + do { + rule = list_node_data_get(node); + CU_DEBUG("Processing %s", rule->name); s = instance_from_rule(_BROKER, info->context, reference, - filter->rules[i], + rule, &instance); if (instance != NULL) { inst_list_add(list, instance); instance = NULL; } - } + node = list_node_next_node(node); + } while (node != head); + + end: cleanup_filters(&filter, 1); out: @@ -104,8 +113,7 @@ static CMPIStatus rule_to_list( CMPIInstance *instance = NULL; const char *name = NULL; virConnectPtr conn = NULL; - int count = 0; - int i, j = 0; + int i, count = 0; CU_DEBUG("Reference = %s", REF2STR(reference)); @@ -126,21 +134,18 @@ static CMPIStatus rule_to_list( /* return the filter that contains the rule */ for (i = 0; i < count; i++) { - for (j = 0; j < filters[i].rule_ct; j++) { - if (STREQC(name, filters[i].rules[j]->name)) { - CU_DEBUG("Processing %s,",filters[i].name); - - s = instance_from_filter(_BROKER, - info->context, - reference, - &filters[i], - &instance); - - if (instance != NULL) { - inst_list_add(list, instance); - instance = NULL; - } - + if (list_find(filters[i].rules, (void *) name) != NULL) { + CU_DEBUG("Processing %s,",filters[i].name); + + s = instance_from_filter(_BROKER, + info->context, + reference, + &filters[i], + &instance); + + if (instance != NULL) { + inst_list_add(list, instance); + instance = NULL; } } } diff --git a/src/Virt_FilterEntry.c b/src/Virt_FilterEntry.c index 126615b..70f8142 100644 --- a/src/Virt_FilterEntry.c +++ b/src/Virt_FilterEntry.c @@ -596,8 +596,10 @@ CMPIStatus enum_filter_rules( { virConnectPtr conn = NULL; struct acl_filter *filters = NULL; - int i, j, count = 0; + int i, count = 0; CMPIStatus s = {CMPI_RC_OK, NULL}; + list_node_t *head, *node; + CU_DEBUG("Reference = %s", REF2STR(reference)); @@ -618,11 +620,14 @@ CMPIStatus enum_filter_rules( count = get_filters(conn, &filters); for (i = 0; i < count; i++) { - for (j = 0; j < filters[i].rule_ct; j++) { - CMPIInstance *instance = NULL; + CMPIInstance *instance = NULL; + head = node = list_first_node(filters[i].rules); + if (head == NULL) + continue; + do { instance = convert_rule_to_instance( - filters[i].rules[j], + list_node_data_get(node), broker, context, reference, @@ -630,8 +635,9 @@ CMPIStatus enum_filter_rules( if (instance != NULL) inst_list_add(list, instance); - } + node = list_node_next_node(node); + } while (node != head); } out: @@ -652,9 +658,7 @@ CMPIStatus get_rule_by_ref( struct acl_rule *rule = NULL; const char *name = NULL; char *filter_name = NULL; - int rule_index; virConnectPtr conn = NULL; - int i; CU_DEBUG("Reference = %s", REF2STR(reference)); @@ -665,15 +669,6 @@ CMPIStatus get_rule_by_ref( goto out; } - if (parse_rule_id(name, &filter_name, &rule_index) == 0) { - cu_statusf(_BROKER, &s, - CMPI_RC_ERR_NOT_FOUND, - "Could not parse filter name"); - goto out; - } - - CU_DEBUG("Filter name = %s, rule index = %u", filter_name, rule_index); - conn = connect_by_classname(_BROKER, CLASSNAME(reference), &s); if (conn == NULL) goto out; @@ -686,13 +681,7 @@ CMPIStatus get_rule_by_ref( goto out; } - for (i = 0; i < filter->rule_ct; i++) { - if (rule_index == i) { - rule = filter->rules[i]; - break; - } - } - + rule = list_find(filter->rules, (void *) name); if (rule == NULL) { cu_statusf(_BROKER, &s, CMPI_RC_ERR_NOT_FOUND, diff --git a/src/Virt_NestedFilterList.c b/src/Virt_NestedFilterList.c index a8565d6..3e9fcc3 100644 --- a/src/Virt_NestedFilterList.c +++ b/src/Virt_NestedFilterList.c @@ -141,9 +141,6 @@ static CMPIStatus parent_to_child( goto out; /* Walk refs list */ - if (parent_filter->refs == NULL) - goto end; - head = node = list_first_node(parent_filter->refs); if (head == NULL) goto end;

From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com&gt; Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com&gt; --- 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)