+1 run cimtest before pushing -Sharad On Tue, 2012-01-31 at 19:58 -0200, Eduardo Lima (Etrunko) wrote:

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

Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- libxkutil/Makefile.am | 51 +++++++--- libxkutil/list_util.c | 254 +++++++++++++++++++++++++++++++++++++++++++++++++ libxkutil/list_util.h | 73 ++++++++++++++ 3 files changed, 364 insertions(+), 14 deletions(-) create mode 100644 libxkutil/list_util.c create mode 100644 libxkutil/list_util.h

diff --git a/libxkutil/Makefile.am b/libxkutil/Makefile.am index f1adc03..8d436ad 100644 --- a/libxkutil/Makefile.am +++ b/libxkutil/Makefile.am @@ -1,21 +1,44 @@ # Copyright IBM Corp. 2007 +AM_CFLAGS = \ + $(CFLAGS_STRICT) \ + -DLIBVIRTCIM_CONF=\"@sysconfdir@/@PACKAGE@.conf\"

-AM_CFLAGS = $(CFLAGS_STRICT) \ - -DLIBVIRTCIM_CONF=\"@sysconfdir@/@PACKAGE@.conf\" +noinst_HEADERS = \ + cs_util.h \ + misc_util.h \ + device_parsing.h \ + xmlgen.h \ + infostore.h \ + pool_parsing.h \ + acl_parsing.h \ + list_util.h

-noinst_HEADERS = cs_util.h misc_util.h device_parsing.h xmlgen.h infostore.h \ - pool_parsing.h acl_parsing.h +lib_LTLIBRARIES = \ + libxkutil.la

-lib_LTLIBRARIES = libxkutil.la +libxkutil_la_SOURCES = \ + cs_util_instance.c \ + misc_util.c \ + device_parsing.c \ + xmlgen.c \ + infostore.c \ + pool_parsing.c \ + acl_parsing.c \ + list_util.c

-libxkutil_la_SOURCES = cs_util_instance.c misc_util.c device_parsing.c \ - xmlgen.c infostore.c pool_parsing.c acl_parsing.c -libxkutil_la_LDFLAGS = -version-info @VERSION_INFO@ -libxkutil_la_LIBADD = @LIBVIRT_LIBS@ \ - @LIBUUID_LIBS@ +libxkutil_la_LDFLAGS = \ + -version-info @VERSION_INFO@

-noinst_PROGRAMS = xml_parse_test +libxkutil_la_LIBADD = \ + @LIBVIRT_LIBS@ \ + @LIBUUID_LIBS@

-xml_parse_test_SOURCES = xml_parse_test.c -xml_parse_test_LDADD = libxkutil.la \ - @LIBVIRT_LIBS@ +noinst_PROGRAMS = \ + xml_parse_test + +xml_parse_test_SOURCES = \ + xml_parse_test.c + +xml_parse_test_LDADD = \ + libxkutil.la \ + @LIBVIRT_LIBS@ diff --git a/libxkutil/list_util.c b/libxkutil/list_util.c new file mode 100644 index 0000000..84b2ba0 --- /dev/null +++ b/libxkutil/list_util.c @@ -0,0 +1,254 @@ +/* + * Copyright IBM Corp. 2012 + * + * Authors: + * Eduardo Lima (Etrunko) <eblima@br.ibm.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ +#ifdef HAVE_CONFIG_H +# include "config.h" +#endif + +#include <stdlib.h> + +#include "list_util.h" + +struct _list_node_t { + list_node_t *prev; + list_node_t *next; + void *data; +}; + +struct _list_t { + unsigned int count; + list_node_t *head; + list_data_free_cb free_cb; + list_data_cmp_cb cmp_cb; +}; + +list_t *list_new(list_data_free_cb free_cb, list_data_cmp_cb cmp_cb) +{ + list_t *l = calloc(1, sizeof(*l)); + if (l == NULL) + return NULL; + + l->free_cb = free_cb; + l->cmp_cb = cmp_cb; + return l; +} + +void list_free(list_t *list) +{ + list_node_t *n, *next; + + if (list == NULL || list->head == NULL) + return; + + n = list->head; + + do { + if (list->free_cb) + list->free_cb(n->data); + + next = n->next; + free(n); + n = next; + } while (n != list->head); + + free(list); +} + +void list_append(list_t *list, void *data) +{ + list_node_t *n; + + if (list == NULL) + return; + + n = calloc(1, sizeof(*n)); + + if (n == NULL) + return; + + n->data = data; + + if (list->head == NULL) { /* empty list */ + n->next = n->prev = n; + list->head = n; + goto end; + } + + n->next = list->head; + n->prev = list->head->prev; + + list->head->prev->next = n; + list->head->prev = n; + + end: + list->count += 1; +} + +void list_prepend(list_t *list, void *data) +{ + list_append(list, data); + list->head = list->head->prev; +} + +void *list_find(list_t *list, void *user_data) +{ + list_node_t *n = list_find_node(list, user_data); + return list_node_data_get(n); +} + +list_node_t *list_find_node(list_t *list, void *user_data) +{ + list_node_t *n; + + if (list == NULL || list->head == NULL || list->cmp_cb == NULL) + return NULL; + + n = list->head; + do { + if (list->cmp_cb(n->data, user_data) == 0) + return n; + + n = n->next; + } while (n != list->head); + + return NULL; +} + +void list_remove(list_t *list, void *data) +{ + list_node_t *n = list_find_node(list, data); + list_remove_node(list, n); +} + +void list_remove_node(list_t *list, list_node_t *node) +{ + if (list == NULL || list->head == NULL || node == NULL) + return; + + if (node->next == node) { /* only 1 item */ + list->head = NULL; + } else { + if (node == list->head) /* first node */ + list->head = node->next; + + node->prev->next = node->next; + node->next->prev = node->prev; + } + + if (list->free_cb) + list->free_cb(node->data); + + free(node); + list->count -= 1; +} + +bool list_foreach(list_t *list, list_foreach_cb cb, void *user_data) +{ + list_node_t *node; + + if (list == NULL || list->head == NULL) + return true; /* nothing to do */ + + node = list->head; + do { + if (cb(node->data, user_data) == false) + return false; + + node = node->next; + } while (node != list->head); + + return true; +} + +unsigned int list_count(list_t *list) +{ + if (list == NULL) + return 0; + + return list->count; +} + +void *list_node_data_get(list_node_t *node) +{ + if (node == NULL) + return NULL; + + return node->data; +} + +void list_node_data_set(list_node_t *node, void *data) +{ + if (node == NULL) + return; + + node->data = data; +} + +void *list_first(list_t *list) +{ + return list_node_data_get(list_first_node(list)); +} + +list_node_t *list_first_node(list_t *list) +{ + if (list == NULL) + return NULL; + + return list->head; +} + +void *list_last(list_t *list) +{ + return list_node_data_get(list_last_node(list)); +} + +list_node_t *list_last_node(list_t *list) +{ + if (list == NULL || list->head == NULL) + return NULL; + + return list->head->prev; +} + +void *list_node_next(list_node_t *node) +{ + return list_node_data_get(list_node_next_node(node)); +} + +list_node_t *list_node_next_node(list_node_t *node) +{ + if (node == NULL) + return NULL; + + return node->next; +} + +void *list_node_prev(list_node_t *node) +{ + return list_node_data_get(list_node_prev_node(node)); +} + +list_node_t *list_node_prev_node(list_node_t *node) +{ + if (node == NULL) + return NULL; + + return node->prev; +} diff --git a/libxkutil/list_util.h b/libxkutil/list_util.h new file mode 100644 index 0000000..1809c2e --- /dev/null +++ b/libxkutil/list_util.h @@ -0,0 +1,73 @@ +/* + * Copyright IBM Corp. 2012 + * + * Authors: + * Eduardo Lima (Etrunko) <eblima@br.ibm.com> + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA + */ + +#ifndef __LIST_UTIL_H +#define __LIST_UTIL_H + +#include <stdbool.h> + +#ifdef __cplusplus +extern "C" { +#endif + +typedef void (*list_data_free_cb)(void *data); +typedef int (*list_data_cmp_cb)(void *list_data, void *user_data); +typedef bool (*list_foreach_cb)(void *list_data, void *user_data); + +typedef struct _list_node_t list_node_t; +typedef struct _list_t list_t; + +list_t *list_new(list_data_free_cb free_cb, list_data_cmp_cb cmp_cb); +void list_free(list_t *list); + +void list_append(list_t *list, void *data); +void list_prepend(list_t *list, void *data); + +void *list_find(list_t *list, void *user_data); +list_node_t *list_find_node(list_t *list, void *user_data); + +void list_remove(list_t *list, void *data); +void list_remove_node(list_t *list, list_node_t *node); + +bool list_foreach(list_t *list, list_foreach_cb cb, void *user_data); + +inline unsigned int list_count(list_t *list); + +inline void *list_node_data_get(list_node_t *node); +inline void list_node_data_set(list_node_t *node, void *data); + +inline void *list_first(list_t *list); +inline list_node_t *list_first_node(list_t *list); + +inline void *list_last(list_t *list); +inline list_node_t *list_last_node(list_t *list); + +inline void *list_node_next(list_node_t *node); +inline list_node_t *list_node_next_node(list_node_t *node); + +inline void *list_node_prev(list_node_t *node); +inline list_node_t *list_node_prev_node(list_node_t *node); + +#ifdef __cplusplus +} /* extern "C" */ +#endif + +#endif /* __LIST_UTIL_H */

+1 run cimtest before pushing -Sharad On Tue, 2012-01-31 at 19:58 -0200, Eduardo Lima (Etrunko) wrote:

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

Signed-off-by: Eduardo Lima (Etrunko) <eblima@br.ibm.com> --- src/Virt_ComputerSystemIndication.c | 95 ++++++++++------------------------- 1 files changed, 26 insertions(+), 69 deletions(-)

diff --git a/src/Virt_ComputerSystemIndication.c b/src/Virt_ComputerSystemIndication.c index 6ef2ddc..712e12c 100644 --- a/src/Virt_ComputerSystemIndication.c +++ b/src/Virt_ComputerSystemIndication.c @@ -19,6 +19,10 @@ * License along with this library; if not, write to the Free Software * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ +#ifdef HAVE_CONFIG_H +# include "config.h" +#endif + #include <unistd.h> #include <stdio.h> #include <fcntl.h> @@ -34,11 +38,11 @@ #include <libvirt/virterror.h>

#include <libcmpiutil/libcmpiutil.h> -#include <misc_util.h> #include <libcmpiutil/std_indication.h> -#include <cs_util.h>

-#include "config.h" +#include <misc_util.h> +#include <cs_util.h> +#include <list_util.h>

#include "Virt_ComputerSystem.h" #include "Virt_ComputerSystemIndication.h" @@ -64,8 +68,6 @@ struct _csi_dom_xml_t { char uuid[VIR_UUID_STRING_BUFLEN]; char *name; char *xml; - csi_dom_xml_t *next; - csi_dom_xml_t *prev; };

typedef struct _csi_thread_data_t csi_thread_data_t; @@ -73,7 +75,7 @@ struct _csi_thread_data_t { CMPI_THREAD_TYPE id; int active_filters; int dom_count; - csi_dom_xml_t *dom_list; + list_t *dom_list; struct ind_args *args; };

@@ -83,15 +85,24 @@ static bool lifecycle_enabled = false; static csi_thread_data_t csi_thread_data[CSI_NUM_PLATFORMS] = {{0}, {0}, {0}};

/* - * Domain list manipulation + * Domain manipulation */ -static void csi_dom_xml_free(csi_dom_xml_t *dom) +static void csi_dom_xml_free(void *data) { + csi_dom_xml_t *dom = (csi_dom_xml_t *) data; free(dom->xml); free(dom->name); free(dom); }

+static int csi_dom_xml_cmp(void *data, void *cmp_cb_data) +{ + csi_dom_xml_t *dom = (csi_dom_xml_t *) data; + const char *uuid = (const char *) cmp_cb_data; + + return strcmp(dom->uuid, uuid); +} + static int csi_dom_xml_set(csi_dom_xml_t *dom, virDomainPtr dom_ptr, CMPIStatus *s) { const char *name; @@ -150,65 +161,10 @@ static csi_dom_xml_t *csi_dom_xml_new(virDomainPtr dom_ptr, CMPIStatus *s) static void csi_thread_dom_list_append(csi_thread_data_t *thread, csi_dom_xml_t *dom) { - /* empty list */ - if (thread->dom_list == NULL) { - dom->next = dom->prev = dom; - thread->dom_list = dom; - goto end; - } - - dom->next = thread->dom_list; - dom->prev = thread->dom_list->prev; - - thread->dom_list->prev->next = dom; - thread->dom_list->prev = dom; - - end: - thread->dom_count += 1; -} - -static csi_dom_xml_t *csi_thread_dom_list_find(csi_thread_data_t *thread, - const char *uuid) -{ - csi_dom_xml_t *dom; - if (thread->dom_list == NULL) - return NULL; - - dom = thread->dom_list; - - do { - if (STREQ(dom->uuid, uuid)) - return dom; - - dom = dom->next; - } while (dom != thread->dom_list); + thread->dom_list = list_new(csi_dom_xml_free, csi_dom_xml_cmp);

- return NULL; -} - -static void csi_thread_dom_list_remove(csi_thread_data_t *thread, - csi_dom_xml_t *dom) -{ - if (dom->next == dom) { /* Only one node */ - thread->dom_list = NULL; - } else { - if (thread->dom_list == dom) /* First node */ - thread->dom_list = dom->next; - - dom->prev->next = dom->next; - dom->next->prev = dom->prev; - } - - thread->dom_count -= 1; - - csi_dom_xml_free(dom); -} - -static void csi_thread_dom_list_free(csi_thread_data_t *thread) -{ - while(thread->dom_list != NULL) - csi_thread_dom_list_remove(thread, thread->dom_list); + list_append(thread->dom_list, dom); }

static void csi_free_thread_data(void *data) @@ -218,7 +174,8 @@ static void csi_free_thread_data(void *data) if (data == NULL) return;

- csi_thread_dom_list_free(thread); + list_free(thread->dom_list); + thread->dom_list = NULL; stdi_free_ind_args(&thread->args); }

@@ -512,7 +469,7 @@ static int update_domain_list(virConnectPtr conn, csi_thread_data_t *thread) CMPIStatus s = {CMPI_RC_OK, NULL}; int i, count;

- csi_thread_dom_list_free(thread); + list_free(thread->dom_list);

count = get_domain_list(conn, &dom_ptr_list);

@@ -574,7 +531,7 @@ static int csi_domain_event_cb(virConnectPtr conn, if (cs_event != CS_CREATED) { char uuid[VIR_UUID_STRING_BUFLEN] = {0}; virDomainGetUUIDString(dom, &uuid[0]); - dom_xml = csi_thread_dom_list_find(thread, uuid); + dom_xml = list_find(thread->dom_list, uuid); }

if (dom_xml == NULL) { @@ -595,7 +552,7 @@ static int csi_domain_event_cb(virConnectPtr conn, } } else if (event == VIR_DOMAIN_EVENT_DEFINED && detail == VIR_DOMAIN_EVENT_UNDEFINED_REMOVED) { - csi_thread_dom_list_remove(thread, dom_xml); + list_remove(thread->dom_list, dom_xml); }

end:

On Tue, 2012-01-31 at 19:58 -0200, 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 | 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))

why call list_foreach(), why not call filter_ref_foreach() directly?

+ 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);

+1 run cimtest before pushing -Sharad On Tue, 2012-01-31 at 19:58 -0200, 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 | 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);