10:35 a.m.
From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com>
This series include a fix to the list code which was leaking the list pointer
in the case where it was empty. Also ports the acl and pool libxkutil helpers
to make use of the list implementation. Resulting in much cleaner and smaller
code.
Eduardo Lima (Etrunko) (3):
list_util: Fix possible memory leak
ACL: Port filter rules to use list implementation
Make pool device paths use linked list implementation
libxkutil/acl_parsing.c | 71 ++++++---------------------
libxkutil/acl_parsing.h | 5 +-
libxkutil/list_util.c | 6 ++-
libxkutil/pool_parsing.c | 27 +++-------
libxkutil/pool_parsing.h | 4 +-
libxkutil/xmlgen.c | 29 ++++++-----
src/Virt_EntriesInFilterList.c | 49 +++++++++---------
src/Virt_FilterEntry.c | 35 +++++--------
src/Virt_NestedFilterList.c | 3 --
src/Virt_ResourcePoolConfigurationService.c | 28 ++++-------
10 files changed, 96 insertions(+), 161 deletions(-)
--
1.7.10.4
10:35 a.m.
New subject: [PATCH 1/3] list_util: Fix possible memory leak
From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com>
The list pointer was leaking in the case where the list was empty.
Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
---
libxkutil/list_util.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libxkutil/list_util.c b/libxkutil/list_util.c
index 84b2ba0..cab0287 100644
--- a/libxkutil/list_util.c
+++ b/libxkutil/list_util.c
@@ -54,9 +54,12 @@ void list_free(list_t *list)
{
list_node_t *n, *next;
- if (list == NULL || list->head == NULL)
+ if (list == NULL)
return;
+ if (list->head == NULL)
+ goto end;
+
n = list->head;
do {
@@ -68,6 +71,7 @@ void list_free(list_t *list)
n = next;
} while (n != list->head);
+ end:
free(list);
}
--
1.7.10.4
10:35 a.m.
New subject: [PATCH 2/3] ACL: Port filter rules to use list implementation
From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com>
Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)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;
--
1.7.10.4
10:35 a.m.
New subject: [PATCH 3/3] Make pool device paths use linked list implementation
From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com>
Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
---
libxkutil/pool_parsing.c | 27 ++++++-------------------
libxkutil/pool_parsing.h | 4 ++--
libxkutil/xmlgen.c | 29 +++++++++++++++++----------
src/Virt_ResourcePoolConfigurationService.c | 28 +++++++++-----------------
4 files changed, 35 insertions(+), 53 deletions(-)
diff --git a/libxkutil/pool_parsing.c b/libxkutil/pool_parsing.c
index e41fc09..bb616d4 100644
--- a/libxkutil/pool_parsing.c
+++ b/libxkutil/pool_parsing.c
@@ -54,8 +54,6 @@ static void cleanup_net_pool(struct net_pool pool) {
}
static void cleanup_disk_pool(struct disk_pool pool) {
- uint16_t i;
-
free(pool.path);
free(pool.host);
free(pool.src_dir);
@@ -63,10 +61,7 @@ static void cleanup_disk_pool(struct disk_pool pool) {
free(pool.port_name);
free(pool.node_name);
- for (i = 0; i < pool.device_paths_ct; i++)
- free(pool.device_paths[i]);
-
- free(pool.device_paths);
+ list_free(pool.device_paths);
}
void cleanup_virt_pool(struct virt_pool **pool)
@@ -130,22 +125,15 @@ static int parse_disk_target(xmlNode *node, struct disk_pool *pool)
static int parse_disk_source(xmlNode *node, struct disk_pool *pool)
{
xmlNode *child;
- char **dev_paths = NULL;
- unsigned ct = 0;
for (child = node->children; child != NULL; child = child->next) {
if (XSTREQ(child->name, "device")) {
- char **tmp = NULL;
-
- tmp = realloc(dev_paths, sizeof(char *) * (ct + 1));
- if (tmp == NULL) {
- CU_DEBUG("Could not alloc space for dev
path");
- continue;
- }
- dev_paths = tmp;
+ if (pool->device_paths == NULL)
+ pool->device_paths = list_new(free,
+ (list_data_cmp_cb) strcmp);
- dev_paths[ct] = get_attr_value(child, "path");
- ct++;
+ list_append(pool->device_paths,
+ get_attr_value(child, "path"));
} else if (XSTREQ(child->name, "host")) {
pool->host = get_attr_value(child, "name");
if (pool->host == NULL)
@@ -161,12 +149,9 @@ static int parse_disk_source(xmlNode *node, struct disk_pool *pool)
}
}
- pool->device_paths_ct = ct;
- pool->device_paths = dev_paths;
return 1;
err:
- free(dev_paths);
return 0;
}
diff --git a/libxkutil/pool_parsing.h b/libxkutil/pool_parsing.h
index 9f1a386..50fbeac 100644
--- a/libxkutil/pool_parsing.h
+++ b/libxkutil/pool_parsing.h
@@ -26,6 +26,7 @@
#include <libvirt/libvirt.h>
#include "../src/svpc_types.h"
+#include "list_util.h"
struct net_pool {
char *addr;
@@ -46,8 +47,7 @@ struct disk_pool {
DISK_POOL_LOGICAL,
DISK_POOL_SCSI} pool_type;
char *path;
- char **device_paths;
- uint16_t device_paths_ct;
+ list_t *device_paths;
char *host;
char *src_dir;
char *adapter;
diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c
index 2dcd0d2..26e7203 100644
--- a/libxkutil/xmlgen.c
+++ b/libxkutil/xmlgen.c
@@ -1177,27 +1177,34 @@ static const char *disk_pool_type_to_str(uint16_t type)
return NULL;
}
+static bool device_paths_foreach(void *list_data, void *user_data)
+{
+ char *dev_path = (char *) list_data;
+ xmlNodePtr src = (xmlNodePtr) user_data;
+ xmlNodePtr tmp = NULL;
+
+ tmp = xmlNewChild(src, NULL, BAD_CAST "device", BAD_CAST NULL);
+ if (tmp == NULL)
+ return false;
+
+ if (xmlNewProp(tmp, BAD_CAST "path", BAD_CAST dev_path) == NULL)
+ return false;
+
+ return true;
+}
+
static const char *set_disk_pool_source(xmlNodePtr disk,
struct disk_pool *pool)
{
xmlNodePtr src;
xmlNodePtr tmp;
- uint16_t i;
src = xmlNewChild(disk, NULL, BAD_CAST "source", NULL);
if (src == NULL)
return XML_ERROR;
- for (i = 0; i < pool->device_paths_ct; i++) {
- tmp = xmlNewChild(src, NULL, BAD_CAST "device", BAD_CAST
NULL);
- if (tmp == NULL)
- return XML_ERROR;
-
- if (xmlNewProp(tmp,
- BAD_CAST "path",
- BAD_CAST pool->device_paths[i]) == NULL)
- return XML_ERROR;
- }
+ if (!list_foreach(pool->device_paths, device_paths_foreach, src))
+ return XML_ERROR;
if (pool->host != NULL) {
tmp = xmlNewChild(src, NULL, BAD_CAST "host", BAD_CAST NULL);
diff --git a/src/Virt_ResourcePoolConfigurationService.c
b/src/Virt_ResourcePoolConfigurationService.c
index 751d016..bc3e431 100644
--- a/src/Virt_ResourcePoolConfigurationService.c
+++ b/src/Virt_ResourcePoolConfigurationService.c
@@ -143,7 +143,6 @@ static const char *net_rasd_to_pool(CMPIInstance *inst,
static void init_disk_pool(struct virt_pool *pool)
{
pool->pool_info.disk.device_paths = NULL;
- pool->pool_info.disk.device_paths_ct = 0;
pool->pool_info.disk.path = NULL;
pool->pool_info.disk.host = NULL;
pool->pool_info.disk.src_dir = NULL;
@@ -153,9 +152,8 @@ static void init_disk_pool(struct virt_pool *pool)
pool->pool_info.disk.autostart = 0;
}
-static char *get_dev_paths(CMPIInstance *inst,
- char ***path_list,
- uint16_t *count)
+
+static char *get_dev_paths(CMPIInstance *inst, list_t **path_list)
{
CMPICount i;
CMPICount ct;
@@ -170,11 +168,8 @@ static char *get_dev_paths(CMPIInstance *inst,
if ((s.rc != CMPI_RC_OK) || (ct <= 0))
return "Unable to get DevicePaths array count";
- *path_list = calloc(ct, sizeof(char *));
if (*path_list == NULL)
- return "Failed to alloc space for device paths";
-
- *count = ct;
+ *path_list = list_new(free, (list_data_cmp_cb) strcmp);
for (i = 0; i < ct; i++) {
const char *str = NULL;
@@ -187,7 +182,7 @@ static char *get_dev_paths(CMPIInstance *inst,
if (str == NULL)
return "Unable to get value of DevicePaths element";
- *path_list[i] = strdup(str);
+ list_append(*path_list, strdup(str));
}
return NULL;
@@ -198,10 +193,7 @@ static const char *disk_fs_or_disk_or_logical_pool(CMPIInstance
*inst,
{
const char *msg = NULL;
- msg = get_dev_paths(inst,
- &pool->pool_info.disk.device_paths,
- &pool->pool_info.disk.device_paths_ct);
-
+ msg = get_dev_paths(inst, &pool->pool_info.disk.device_paths);
/* Specifying a value for DevicePaths isn't mandatory for logical
pool types. */
@@ -209,7 +201,7 @@ static const char *disk_fs_or_disk_or_logical_pool(CMPIInstance
*inst,
if (msg != NULL)
return msg;
- if (pool->pool_info.disk.device_paths_ct != 1) {
+ if (list_count(pool->pool_info.disk.device_paths) != 1) {
CU_DEBUG("%d only takes one device path",
pool->pool_info.disk.pool_type);
return "Specified pool type only takes one device
path";
@@ -243,14 +235,12 @@ static const char *disk_iscsi_pool(CMPIInstance *inst,
const char *val = NULL;
const char *msg = NULL;
- msg = get_dev_paths(inst,
- &pool->pool_info.disk.device_paths,
- &pool->pool_info.disk.device_paths_ct);
-
+ msg = get_dev_paths(inst, &pool->pool_info.disk.device_paths);
+
if (msg != NULL)
return msg;
- if (pool->pool_info.disk.device_paths_ct != 1)
+ if (list_count(pool->pool_info.disk.device_paths) != 1)
return "Specified pool type only takes one device path";
if (cu_get_str_prop(inst, "Host", &val) != CMPI_RC_OK)
--
1.7.10.4
1:32 a.m.
New subject: [PATCH 1/3] list_util: Fix possible memory leak
On Tue, Jun 19, 2012 at 12:35:38PM -0300, Eduardo Lima (Etrunko) wrote:
From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com>
The list pointer was leaking in the case where the list was empty.
Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
---
libxkutil/list_util.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libxkutil/list_util.c b/libxkutil/list_util.c
index 84b2ba0..cab0287 100644
--- a/libxkutil/list_util.c
+++ b/libxkutil/list_util.c
@@ -54,9 +54,12 @@ void list_free(list_t *list)
{
list_node_t *n, *next;
- if (list == NULL || list->head == NULL)
+ if (list == NULL)
return;
+ if (list->head == NULL)
+ goto end;
+
n = list->head;
do {
@@ -68,6 +71,7 @@ void list_free(list_t *list)
n = next;
} while (n != list->head);
+ end:
free(list);
}
Patch looks reasonable based on the description, ACK
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
2:08 a.m.
New subject: [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(a)br.ibm.com>
Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)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(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
2:11 a.m.
New subject: [PATCH 3/3] Make pool device paths use linked list implementation
On Tue, Jun 19, 2012 at 12:35:40PM -0300, Eduardo Lima (Etrunko) wrote:
From: "Eduardo Lima (Etrunko)" <eblima(a)br.ibm.com>
Signed-off-by: Eduardo Lima (Etrunko) <eblima(a)br.ibm.com>
---
libxkutil/pool_parsing.c | 27 ++++++-------------------
libxkutil/pool_parsing.h | 4 ++--
libxkutil/xmlgen.c | 29 +++++++++++++++++----------
src/Virt_ResourcePoolConfigurationService.c | 28 +++++++++-----------------
4 files changed, 35 insertions(+), 53 deletions(-)
diff --git a/libxkutil/pool_parsing.c b/libxkutil/pool_parsing.c
index e41fc09..bb616d4 100644
--- a/libxkutil/pool_parsing.c
+++ b/libxkutil/pool_parsing.c
@@ -54,8 +54,6 @@ static void cleanup_net_pool(struct net_pool pool) {
}
static void cleanup_disk_pool(struct disk_pool pool) {
- uint16_t i;
-
free(pool.path);
free(pool.host);
free(pool.src_dir);
@@ -63,10 +61,7 @@ static void cleanup_disk_pool(struct disk_pool pool) {
free(pool.port_name);
free(pool.node_name);
- for (i = 0; i < pool.device_paths_ct; i++)
- free(pool.device_paths[i]);
-
- free(pool.device_paths);
+ list_free(pool.device_paths);
}
void cleanup_virt_pool(struct virt_pool **pool)
@@ -130,22 +125,15 @@ static int parse_disk_target(xmlNode *node, struct disk_pool
*pool)
static int parse_disk_source(xmlNode *node, struct disk_pool *pool)
{
xmlNode *child;
- char **dev_paths = NULL;
- unsigned ct = 0;
for (child = node->children; child != NULL; child = child->next) {
if (XSTREQ(child->name, "device")) {
- char **tmp = NULL;
-
- tmp = realloc(dev_paths, sizeof(char *) * (ct + 1));
- if (tmp == NULL) {
- CU_DEBUG("Could not alloc space for dev
path");
- continue;
- }
- dev_paths = tmp;
+ if (pool->device_paths == NULL)
+ pool->device_paths = list_new(free,
+ (list_data_cmp_cb) strcmp);
- dev_paths[ct] = get_attr_value(child, "path");
- ct++;
+ list_append(pool->device_paths,
+ get_attr_value(child, "path"));
} else if (XSTREQ(child->name, "host")) {
pool->host = get_attr_value(child, "name");
if (pool->host == NULL)
@@ -161,12 +149,9 @@ static int parse_disk_source(xmlNode *node, struct disk_pool *pool)
}
}
- pool->device_paths_ct = ct;
- pool->device_paths = dev_paths;
return 1;
err:
- free(dev_paths);
return 0;
}
diff --git a/libxkutil/pool_parsing.h b/libxkutil/pool_parsing.h
index 9f1a386..50fbeac 100644
--- a/libxkutil/pool_parsing.h
+++ b/libxkutil/pool_parsing.h
@@ -26,6 +26,7 @@
#include <libvirt/libvirt.h>
#include "../src/svpc_types.h"
+#include "list_util.h"
struct net_pool {
char *addr;
@@ -46,8 +47,7 @@ struct disk_pool {
DISK_POOL_LOGICAL,
DISK_POOL_SCSI} pool_type;
char *path;
- char **device_paths;
- uint16_t device_paths_ct;
+ list_t *device_paths;
char *host;
char *src_dir;
char *adapter;
diff --git a/libxkutil/xmlgen.c b/libxkutil/xmlgen.c
index 2dcd0d2..26e7203 100644
--- a/libxkutil/xmlgen.c
+++ b/libxkutil/xmlgen.c
@@ -1177,27 +1177,34 @@ static const char *disk_pool_type_to_str(uint16_t type)
return NULL;
}
+static bool device_paths_foreach(void *list_data, void *user_data)
+{
+ char *dev_path = (char *) list_data;
+ xmlNodePtr src = (xmlNodePtr) user_data;
+ xmlNodePtr tmp = NULL;
+
+ tmp = xmlNewChild(src, NULL, BAD_CAST "device", BAD_CAST NULL);
+ if (tmp == NULL)
+ return false;
+
+ if (xmlNewProp(tmp, BAD_CAST "path", BAD_CAST dev_path) == NULL)
+ return false;
+
+ return true;
+}
+
static const char *set_disk_pool_source(xmlNodePtr disk,
struct disk_pool *pool)
{
xmlNodePtr src;
xmlNodePtr tmp;
- uint16_t i;
src = xmlNewChild(disk, NULL, BAD_CAST "source", NULL);
if (src == NULL)
return XML_ERROR;
- for (i = 0; i < pool->device_paths_ct; i++) {
- tmp = xmlNewChild(src, NULL, BAD_CAST "device", BAD_CAST
NULL);
- if (tmp == NULL)
- return XML_ERROR;
-
- if (xmlNewProp(tmp,
- BAD_CAST "path",
- BAD_CAST pool->device_paths[i]) == NULL)
- return XML_ERROR;
- }
+ if (!list_foreach(pool->device_paths, device_paths_foreach, src))
+ return XML_ERROR;
if (pool->host != NULL) {
tmp = xmlNewChild(src, NULL, BAD_CAST "host", BAD_CAST NULL);
diff --git a/src/Virt_ResourcePoolConfigurationService.c
b/src/Virt_ResourcePoolConfigurationService.c
index 751d016..bc3e431 100644
--- a/src/Virt_ResourcePoolConfigurationService.c
+++ b/src/Virt_ResourcePoolConfigurationService.c
@@ -143,7 +143,6 @@ static const char *net_rasd_to_pool(CMPIInstance *inst,
static void init_disk_pool(struct virt_pool *pool)
{
pool->pool_info.disk.device_paths = NULL;
- pool->pool_info.disk.device_paths_ct = 0;
pool->pool_info.disk.path = NULL;
pool->pool_info.disk.host = NULL;
pool->pool_info.disk.src_dir = NULL;
@@ -153,9 +152,8 @@ static void init_disk_pool(struct virt_pool *pool)
pool->pool_info.disk.autostart = 0;
}
-static char *get_dev_paths(CMPIInstance *inst,
- char ***path_list,
- uint16_t *count)
+
+static char *get_dev_paths(CMPIInstance *inst, list_t **path_list)
{
CMPICount i;
CMPICount ct;
@@ -170,11 +168,8 @@ static char *get_dev_paths(CMPIInstance *inst,
if ((s.rc != CMPI_RC_OK) || (ct <= 0))
return "Unable to get DevicePaths array count";
- *path_list = calloc(ct, sizeof(char *));
if (*path_list == NULL)
- return "Failed to alloc space for device paths";
-
- *count = ct;
+ *path_list = list_new(free, (list_data_cmp_cb) strcmp);
for (i = 0; i < ct; i++) {
const char *str = NULL;
@@ -187,7 +182,7 @@ static char *get_dev_paths(CMPIInstance *inst,
if (str == NULL)
return "Unable to get value of DevicePaths element";
- *path_list[i] = strdup(str);
+ list_append(*path_list, strdup(str));
}
return NULL;
@@ -198,10 +193,7 @@ static const char *disk_fs_or_disk_or_logical_pool(CMPIInstance
*inst,
{
const char *msg = NULL;
- msg = get_dev_paths(inst,
- &pool->pool_info.disk.device_paths,
- &pool->pool_info.disk.device_paths_ct);
-
+ msg = get_dev_paths(inst, &pool->pool_info.disk.device_paths);
/* Specifying a value for DevicePaths isn't mandatory for logical
pool types. */
@@ -209,7 +201,7 @@ static const char *disk_fs_or_disk_or_logical_pool(CMPIInstance
*inst,
if (msg != NULL)
return msg;
- if (pool->pool_info.disk.device_paths_ct != 1) {
+ if (list_count(pool->pool_info.disk.device_paths) != 1) {
CU_DEBUG("%d only takes one device path",
pool->pool_info.disk.pool_type);
return "Specified pool type only takes one device
path";
@@ -243,14 +235,12 @@ static const char *disk_iscsi_pool(CMPIInstance *inst,
const char *val = NULL;
const char *msg = NULL;
- msg = get_dev_paths(inst,
- &pool->pool_info.disk.device_paths,
- &pool->pool_info.disk.device_paths_ct);
-
+ msg = get_dev_paths(inst, &pool->pool_info.disk.device_paths);
+
if (msg != NULL)
return msg;
- if (pool->pool_info.disk.device_paths_ct != 1)
+ if (list_count(pool->pool_info.disk.device_paths) != 1)
return "Specified pool type only takes one device path";
if (cu_get_str_prop(inst, "Host", &val) != CMPI_RC_OK)
Looks fine but I would rename device_paths_foreach() to indicates
what the function does instead of how it is used, something like
device_path_dump_xml() or something similar,
That doesn't affect the code semantic though ACK,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
9:30 p.m.
New subject: [PATCH 1/3] list_util: Fix possible memory leak
+1
δΊ 2012-6-19 23:35, Eduardo Lima (Etrunko) ει:
From: "Eduardo Lima (Etrunko)"<eblima(a)br.ibm.com>
The list pointer was leaking in the case where the list was empty.
Signed-off-by: Eduardo Lima (Etrunko)<eblima(a)br.ibm.com>
---
libxkutil/list_util.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/libxkutil/list_util.c b/libxkutil/list_util.c
index 84b2ba0..cab0287 100644
--- a/libxkutil/list_util.c
+++ b/libxkutil/list_util.c
@@ -54,9 +54,12 @@ void list_free(list_t *list)
{
list_node_t *n, *next;
- if (list == NULL || list->head == NULL)
+ if (list == NULL)
return;
+ if (list->head == NULL)
+ goto end;
+
n = list->head;
do {
@@ -68,6 +71,7 @@ void list_free(list_t *list)
n = next;
} while (n != list->head);
+ end:
free(list);
}
--
Best Regards
Wayne Xia
mail:xiawenc@linux.vnet.ibm.com
tel:86-010-82450803
4414
days inactive
4420
days old
7 comments
3 participants
participants (3)
-
Daniel Veillard -
Eduardo Lima (Etrunko) -
Wayne Xia