On Tue, Jun 19, 2012 at 12:35:39PM -0300, Eduardo Lima (Etrunko) wrote: > From: "Eduardo Lima (Etrunko)" <[email protected]> > > Signed-off-by: Eduardo Lima (Etrunko) <[email protected]> > --- > 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/ [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
