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

Daniel Veillard veillard at redhat.com
Wed Jun 20 07:08:42 UTC 2012


On Tue, Jun 19, 2012 at 12:35:39PM -0300, Eduardo Lima (Etrunko) wrote:
> From: "Eduardo Lima (Etrunko)" <eblima at br.ibm.com>
> 
> Signed-off-by: Eduardo Lima (Etrunko) <eblima at 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 at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/




More information about the Libvirt-cim mailing list