Re: [Libvirt-cim] [PATCH 2/3] ACL: Port filter rules to use list implementation

On Tue, Jun 19, 2012 at 12:35:39PM -0300, Eduardo Lima (Etrunko) wrote:

From: "Eduardo Lima (Etrunko)" <eblima@br.ibm.com>

Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- 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;

That looks reasonnable. The refactoring also decrease code size, Tentative ACK as I don't know the code, Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/