Re: [Libvirt-cim] [PATCH 3/3] ACL: Use linked list helper for filter refs

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 | 58 +++++++++------------------------ libxkutil/acl_parsing.h | 5 ++- libxkutil/xmlgen.c | 30 +++++++++++++---- src/Virt_NestedFilterList.c | 73 +++++++++++++++++++++++------------------- 4 files changed, 82 insertions(+), 84 deletions(-) diff --git a/libxkutil/acl_parsing.c b/libxkutil/acl_parsing.c index 9c4b4b2..6dd481b 100644 --- a/libxkutil/acl_parsing.c +++ b/libxkutil/acl_parsing.c @@ -141,11 +141,7 @@ void cleanup_filter(struct acl_filter *filter) free(filter->rules); filter->rule_ct = 0; - for (i = 0; i < filter->ref_ct; i++) - free(filter->refs[i]); - - free(filter->refs); - filter->ref_ct = 0; + list_free(filter->refs); } void cleanup_filters(struct acl_filter **filters, int count) @@ -610,58 +606,36 @@ int append_filter_rule(struct acl_filter *filter, struct acl_rule *rule) return 1; } -int append_filter_ref(struct acl_filter *filter, char *name) + +static int filter_ref_cmp(void *list_data, void *user_data) { - int i; - char **old_refs = NULL; + return strcmp((const char *)list_data, (const char *) user_data); +} - if ((filter == NULL) || (name == NULL)) +int append_filter_ref(struct acl_filter *filter, char *name) +{ + if (filter == NULL || name == NULL) return 0; - for (i = 0; i < filter->ref_ct; i++) - if (STREQC(filter->refs[i], name)) - return 0; /* already exists */ - - old_refs = filter->refs; - - filter->refs = malloc((filter->ref_ct + 1) * sizeof(char *)); + if (filter->refs == NULL) + filter->refs = list_new(free, filter_ref_cmp); - if (filter->refs == NULL) { - CU_DEBUG("Failed to allocate memory for new ref"); - filter->refs = old_refs; - return 0; + if (list_find(filter->refs, name) != NULL) { + free(name); + return 0; /* already exists */ } - memcpy(filter->refs, old_refs, filter->ref_ct * sizeof(char *)); - - filter->refs[filter->ref_ct] = name; - filter->ref_ct++; - - free(old_refs); + list_append(filter->refs, name); return 1; } int remove_filter_ref(struct acl_filter *filter, const char *name) { - int i; - char **old_refs = NULL; - - if ((filter == NULL) || (name == NULL)) + if (filter == NULL || filter->refs == NULL || name == NULL) return 0; - /* TODO: called infrequently, but needs optimization */ - old_refs = filter->refs; - filter->ref_ct = 0; - - for (i = 0; i < filter->ref_ct; i++) { - if (STREQC(old_refs[i], name)) { - free(old_refs[i]); - } - else if(append_filter_ref(filter, old_refs[i]) == 0) { - return 0; - } - } + list_remove(filter->refs, (void *) name); return 1; } diff --git a/libxkutil/acl_parsing.h b/libxkutil/acl_parsing.h index 5b99175..e49f384 100644 --- a/libxkutil/acl_parsing.h +++ b/libxkutil/acl_parsing.h @@ -26,6 +26,8 @@ #include <libxml/parser.h> #include <libxml/xpath.h> +#include "list_util.h" + struct acl_mac_rule { char *srcmacaddr; char *srcmacmask; @@ -152,8 +154,7 @@ struct acl_filter { struct acl_rule **rules; int rule_ct; - char **refs; - int ref_ct; + list_t *refs; }; void cleanup_rule(struct acl_rule *rule); diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c index 9a2ada9..5c16ebe 100644 --- a/libxkutil/xmlgen.c +++ b/libxkutil/xmlgen.c @@ -28,6 +28,7 @@ #include <libxml/xmlsave.h> #include "xmlgen.h" +#include "list_util.h" #ifndef TEST #include "misc_util.h" @@ -1467,12 +1468,31 @@ char *res_to_xml(struct virt_pool_res *res) { return xml; } +static bool filter_ref_foreach(void *list_data, void *user_data) +{ + char *filter = (char *) list_data; + xmlNodePtr root = (xmlNodePtr) user_data; + xmlNodePtr tmp = NULL; + + tmp = xmlNewChild(root, NULL, BAD_CAST "filterref", NULL); + if (tmp == NULL) { + CU_DEBUG("Error creating filterref node"); + return false; + } + + if (xmlNewProp(tmp, BAD_CAST "filter", BAD_CAST list_data) == NULL) { + CU_DEBUG("Error adding filter attribute '%s'", filter); + return false; + } + + return true; +} + char *filter_to_xml(struct acl_filter *filter) { char *xml = NULL; xmlNodePtr root = NULL; xmlNodePtr tmp = NULL; - int i; root = xmlNewNode(NULL, BAD_CAST "filter"); if (root == NULL) @@ -1494,12 +1514,8 @@ char *filter_to_xml(struct acl_filter *filter) goto out; } - for (i = 0; i < filter->ref_ct; i++) { - tmp = xmlNewChild(root, NULL, BAD_CAST "filterref", NULL); - if (xmlNewProp(tmp, BAD_CAST "filter", - BAD_CAST filter->refs[i]) == NULL) - goto out; - } + if (!list_foreach(filter->refs, filter_ref_foreach, (void *) root)) + goto out; xml = tree_to_xml(root); diff --git a/src/Virt_NestedFilterList.c b/src/Virt_NestedFilterList.c index 81c4408..a8565d6 100644 --- a/src/Virt_NestedFilterList.c +++ b/src/Virt_NestedFilterList.c @@ -34,6 +34,7 @@ #include "acl_parsing.h" #include "misc_util.h" +#include "list_util.h" #include "Virt_FilterList.h" static const CMPIBroker *_BROKER; @@ -120,7 +121,7 @@ static CMPIStatus parent_to_child( CMPIInstance *instance = NULL; const char * name = NULL; virConnectPtr conn = NULL; - int i; + list_node_t *head, *node; CU_DEBUG("Reference = %s", REF2STR(reference)); @@ -139,29 +140,39 @@ static CMPIStatus parent_to_child( if (parent_filter == NULL) goto out; - for (i = 0; i < parent_filter->ref_ct; i++) { - get_filter_by_name(conn, parent_filter->refs[i], - &child_filter); - if (child_filter == NULL) - continue; - - CU_DEBUG("Processing %s,", child_filter->name); - - s = instance_from_filter(_BROKER, - info->context, - reference, - child_filter, - &instance); + /* Walk refs list */ + if (parent_filter->refs == NULL) + goto end; + + head = node = list_first_node(parent_filter->refs); + if (head == NULL) + goto end; + + do { + name = (const char *) list_node_data_get(node); + get_filter_by_name(conn, name, &child_filter); + if (child_filter != NULL) { + CU_DEBUG("Processing %s,", child_filter->name); + + s = instance_from_filter(_BROKER, + info->context, + reference, + child_filter, + &instance); + + if (instance != NULL) { + CU_DEBUG("Adding instance to inst_list"); + inst_list_add(list, instance); + } - if (instance != NULL) { - CU_DEBUG("Adding instance to inst_list"); - inst_list_add(list, instance); + cleanup_filters(&child_filter, 1); } - cleanup_filters(&child_filter, 1); instance = NULL; - } + node = list_node_next_node(node); + } while (node != head); + end: cleanup_filters(&parent_filter, 1); out: @@ -183,7 +194,7 @@ static CMPIStatus child_to_parent( CMPIInstance *instance = NULL; const char *name = NULL; virConnectPtr conn = NULL; - int count, i, j; + int count, i; CU_DEBUG("Reference = %s", REF2STR(reference)); @@ -206,24 +217,20 @@ static CMPIStatus child_to_parent( /* return any filter that has name in refs */ for (i = 0; i < count; i++) { - for (j = 0; j < _list[i].ref_ct; j++) { - if (STREQC(name, _list[i].refs[j])) { - CU_DEBUG("Processing %s,", _list[i].name); + if (list_find_node(_list[i].refs, (void *) name) != NULL) { + CU_DEBUG("Processing %s,", _list[i].name); - s = instance_from_filter(_BROKER, - info->context, - reference, - &_list[i], - &instance); - - if (instance != NULL) - inst_list_add(list, instance); + s = instance_from_filter(_BROKER, + info->context, + reference, + &_list[i], + &instance); + if (instance != NULL) { + inst_list_add(list, instance); instance = NULL; } - } - } cleanup_filters(&_list, count);