[PATCH 0 of 3] [RFC][CU] Add CIMXML EO parser
This is a first stab. Looking for comments and review. It's pretty raw at the moment, but it does work for me. I still need to do nested objects, but if people are happy with it now, we can bring it into the tree and add the nested bit after that (which would be my preference). Keep your eyes peeled for leaks and broken bits, as I'm sure there are some :)
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1202512650 28800 # Node ID 03126c9af41607dcaa8b197736dd5059c108261a # Parent 2337330e5a9bc8e71afd04627166155a33dc53f2 [RFC][CU] Make libcmpiutil include libxml libraries/includes As needed for the CIMXML EO parser Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 2337330e5a9b -r 03126c9af416 acinclude.m4 --- a/acinclude.m4 Fri Feb 08 15:15:25 2008 -0800 +++ b/acinclude.m4 Fri Feb 08 15:17:30 2008 -0800 @@ -92,3 +92,9 @@ AC_DEFUN([CHECK_BROKEN_CMPIFT], ] )]) +AC_DEFUN([CHECK_LIBXML2], + [ + PKG_CHECK_MODULES([LIBXML], [libxml-2.0]) + CPPFLAGS="$CPPFLAGS $LIBXML_CFLAGS" + LDFLAGS="$LDFLAGS $LIBXML_LDFLAGS" + ]) diff -r 2337330e5a9b -r 03126c9af416 configure.ac --- a/configure.ac Fri Feb 08 15:15:25 2008 -0800 +++ b/configure.ac Fri Feb 08 15:17:30 2008 -0800 @@ -50,6 +50,8 @@ AC_ARG_ENABLE([eoparser], AM_CONDITIONAL([build_eoparser],[test x$eoparser = xyes]) +CHECK_LIBXML2 + if test x${eoparser} = xyes; then CFLAGS+=" -DHAVE_EOPARSER" fi
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1202512655 28800 # Node ID f6559a2f55abab85baeabd56faaf7804c3ea30f3 # Parent 03126c9af41607dcaa8b197736dd5059c108261a [RFC][CU] Add EI CIMXML parser This handles strings, bools, integers, and arrays of those. It doesn't yet handle nested objects, as we will need for RASD.HostResource[]. I need to do some further investigation of what the XML for that will look like. I have tested this by taking an existing XML for DefineSystem() and changing the MemResourceAllocationSettingData field to this: <INSTANCE CLASSNAME="Xen_MemResourceAllocationSettingData"> <PROPERTY NAME="CreationClassName" TYPE="string"> <VALUE>Xen_MemResourceAllocationSettingData</VALUE> </PROPERTY> <PROPERTY NAME="InstanceID" TYPE="string"> <VALUE>test/mem</VALUE> </PROPERTY> <PROPERTY NAME="ResourceType" TYPE="uint16"> <VALUE>4</VALUE> </PROPERTY> <PROPERTY NAME="VirtualQuantity" TYPE="uint64"> <VALUE>256 </VALUE> </PROPERTY> <PROPERTY.ARRAY NAME="Foo" TYPE="string"> <VALUE.ARRAY> <VALUE>Bar</VALUE> <VALUE>Baz</VALUE> </VALUE.ARRAY> </PROPERTY.ARRAY> </INSTANCE> (The Foo[] parameter in there is just for testing :) Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r 03126c9af416 -r f6559a2f55ab Makefile.am --- a/Makefile.am Fri Feb 08 15:17:30 2008 -0800 +++ b/Makefile.am Fri Feb 08 15:17:35 2008 -0800 @@ -10,6 +10,8 @@ pkgconfig_DATA = libcmpiutil.pc libcmpiutilincdir = $(includedir)/libcmpiutil +noinst_HEADERS = eo_parser_xml.h + libcmpiutilinc_HEADERS = libcmpiutil.h \ std_invokemethod.h \ std_association.h \ @@ -18,7 +20,7 @@ libcmpiutilinc_HEADERS = libcmpiutil.h \ libcmpiutil_la_SOURCES = args_util.c instance_util.c std_invokemethod.c \ std_association.c inst_list.c std_indication.c \ - debug_util.c + debug_util.c eo_parser_xml.c libcmpiutil_la_CFLAGS = $(CFLAGS) $(CFLAGS_STRICT) libcmpiutil_la_LIBADD = libcmpiutil_la_DEPENDENCIES = diff -r 03126c9af416 -r f6559a2f55ab eo_parser_xml.c --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/eo_parser_xml.c Fri Feb 08 15:17:35 2008 -0800 @@ -0,0 +1,482 @@ +/* + * Copyright IBM Corp. 2007 + * + * Authors: + * Dan Smith <danms@us.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 + */ + +#include <stdio.h> +#include <stdbool.h> +#include <string.h> +#include <stdint.h> +#include <inttypes.h> + +#include <libxml/parser.h> +#include <libxml/tree.h> + +#include <cmpidt.h> +#include <cmpift.h> +#include <cmpimacs.h> + +#include "libcmpiutil.h" + +#include "eo_parser_xml.h" + +static char *get_attr(xmlNode *node, const char *name) +{ + return (char *)xmlGetProp(node, (xmlChar *)name); +} + +static char *get_content(xmlNode *node) +{ + return (char *)xmlNodeGetContent(node); +} + +static CMPIType parse_string_property(const CMPIBroker *broker, + const char *string, + const char *type, + CMPIValue *val) +{ + CMPIString *str; + CMPIStatus s; + + str = CMNewString(broker, string, &s); + if ((str == NULL) || (s.rc != CMPI_RC_OK)) + return CMPI_null; + + val->string = str; + return CMPI_string; +} + +static CMPIType parse_bool_property(const char *string, + const char *type, + CMPIValue *val) +{ + val->boolean = STREQC(string, "true"); + + return CMPI_boolean; +} + +static CMPIType parse_int_property(const char *string, + const char *type, + CMPIValue *val) +{ + int size; + bool sign; + CMPIType t; + int ret; + + if (sscanf(type, "uint%i", &size) == 1) + sign = false; + else if (sscanf(type, "int%i", &size) == 1) + sign = true; + else { + printf("Unknown integer type: `%s'", type); + return 0; + } + + if (sign) { + int64_t _val; + ret = sscanf(string, "%" SCNi64, &_val); + val->sint64 = _val; + } else { + uint64_t _val; + ret = sscanf(string, "%" SCNu64, &_val); + val->uint64 = _val; + } + + if (ret != 1) { + printf("Failed to scan value `%s'\n", string); + return 0; + } + + switch (size) { + case 8: t = sign ? CMPI_sint8 : CMPI_uint8; break; + case 16: t = sign ? CMPI_sint16 : CMPI_uint16; break; + case 32: t = sign ? CMPI_sint32 : CMPI_uint32; break; + default: + case 64: t = sign ? CMPI_sint64 : CMPI_uint64; break; + }; + + return t; +} + +static CMPIType get_property_value(const CMPIBroker *broker, + const char *tstr, + const char *content, + CMPIValue *value) +{ + CMPIType type = CMPI_null; + + if (STREQC(tstr, "string")) + type = parse_string_property(broker, content, tstr, value); + else if (STREQC(tstr, "boolean")) + type = parse_bool_property(content, tstr, value); + else if (strstr(tstr, "int")) + type = parse_int_property(content, tstr, value); + else { + printf("Unhandled type: %s\n", tstr); + goto out; + } + + if (type == CMPI_null) { + printf("Unable to parse type %s\n", tstr); + goto out; + } + + out: + return type; +} + +static CMPIType get_node_value(const CMPIBroker *broker, + xmlNode *node, + const char *tstr, + CMPIValue *val) +{ + CMPIType type = CMPI_null; + char *content = NULL; + + if (node->type != XML_ELEMENT_NODE) { + CU_DEBUG("Non-element node: %s", node->name); + goto out; + } + + if (!STREQC((char *)node->name, "value")) { + CU_DEBUG("Expected <VALUE> but got <%s>", node->name); + goto out; + } + + content = get_content(node); + CU_DEBUG("Node content: %s", content); + type = get_property_value(broker, tstr, content, val); + free(content); + out: + return type; +} + +static CMPIType parse_array(const CMPIBroker *broker, + const char *tstr, + xmlNode *root, + CMPIArray **array) +{ + xmlNode *value; + CMPIValue *list = NULL; + int size = 0; + int cur = 0; + CMPIStatus s; + CMPIType type = CMPI_null; + int i; + + for (value = root->children; value; value = value->next) { + + if (value->type != XML_ELEMENT_NODE) + continue; + + if (cur == size) { + CMPIValue *tmp; + + size += 4; + tmp = realloc(list, sizeof(CMPIValue) * size); + if (tmp == NULL) { + CU_DEBUG("Failed to alloc %i", + sizeof(CMPIValue) * size); + goto out; + } + + list = tmp; + } + + type = get_node_value(broker, value, tstr, &list[cur]); + if (type == CMPI_null) { + CU_DEBUG("Got nothing from child"); + } else { + CU_DEBUG("Array value type %i", type); + cur++; + } + } + + if (cur == 0) + return CMPI_null; + + *array = CMNewArray(broker, cur, type, &s); + if ((*array == NULL) || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("Failed to alloc CMPIArray of %i", cur); + goto out; + } + + for (i = 0; i < cur; i++) + CMSetArrayElementAt((*array), i, &list[i], type); + + out: + free(list); + + return type; +} + +static bool parse_array_property(const CMPIBroker *broker, + xmlNode *node, + CMPIInstance *inst) +{ + char *name = NULL; + char *tstr = NULL; + bool ret = false; + xmlNode *val_arr; + CMPIArray *array; + CMPIType type; + + name = get_attr(node, "NAME"); + if (name == NULL) { + printf("Unnamed property\n"); + goto out; + } + + tstr = get_attr(node, "TYPE"); + if (tstr == NULL) { + printf("No type\n"); + goto out; + } + + printf("Array property `%s' of type `%s'\n", name, tstr); + + for (val_arr = node->children; val_arr; val_arr = val_arr->next) { + if (val_arr->type != XML_ELEMENT_NODE) + continue; + + if (!STREQC((char *)val_arr->name, "value.array")) { + printf("Expected <value.array> but got <%s>", + val_arr->name); + goto out; + } + + break; + } + + type = parse_array(broker, tstr, val_arr, &array); + if (type != CMPI_null) { + CU_DEBUG("Setting array property"); + CMSetProperty(inst, name, &array, (CMPI_ARRAY | type)); + } + + out: + free(name); + free(tstr); + + return ret; +} + +static bool parse_property(const CMPIBroker *broker, + xmlNode *node, + CMPIInstance *inst) +{ + char *name = NULL; + char *tstr = NULL; + CMPIValue value; + CMPIType type = CMPI_null; + xmlNode *ptr; + + name = get_attr(node, "NAME"); + if (name == NULL) { + printf("Unnamed property\n"); + goto out; + } + + tstr = get_attr(node, "TYPE"); + if (tstr == NULL) { + printf("No type\n"); + goto out; + } + + CU_DEBUG("Property %s: %s", name, tstr); + + for (ptr = node->children; ptr; ptr = ptr->next) { + type = get_node_value(broker, ptr, tstr, &value); + if (type != CMPI_null) { + CMSetProperty(inst, name, &value, type); + break; + } + } + out: + free(name); + free(tstr); + + return type != CMPI_null; +} + +static CMPIStatus parse_instance(const CMPIBroker *broker, + const char *ns, + xmlNode *root, + CMPIInstance **inst) +{ + char *class = NULL; + xmlNode *child; + CMPIStatus s; + CMPIObjectPath *op; + + if (root->type != XML_ELEMENT_NODE) { + printf("First node is not <INSTANCE>\n"); + goto out; + } + + if (!STREQC((char *)root->name, "instance")) { + printf("Got node %s, expecting INSTANCE\n", root->name); + goto out; + } + + class = get_attr(root, "CLASSNAME"); + if (class == NULL) { + printf("No Classname\n"); + goto out; + } + + CU_DEBUG("Instance of %s", class); + + op = CMNewObjectPath(broker, ns, class, &s); + if ((op == NULL) || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("Unable to create path for %s:%s", ns, class); + goto out; + } + + *inst = CMNewInstance(broker, op, &s); + if ((*inst == NULL) || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("Unable to create inst for %s:%s", ns, class); + goto out; + } + + for (child = root->children; child; child = child->next) { + if (child->type == XML_ELEMENT_NODE){ + if (STREQC((char *)child->name, "property")) + parse_property(broker, child, *inst); + else if (STREQC((char *)child->name, "property.array")) + parse_array_property(broker, child, *inst); + else + printf("Unexpected node: %s\n", child->name); + } + } + + out: + free(class); + + return s; +} + +#if 0 + +/* This isn't currently used but I think I might need it for nested + * instances, depending on how they look. + */ + +static char *parse_esc(const char *start, char *result) +{ + char *delim; + char *escape = NULL; + int len = 1; + + delim = strchr(start, ';'); + if (delim == NULL) + goto out; + + escape = strndup(start, delim - start + 1); + if (escape == NULL) { + CU_DEBUG("Memory alloc failed (%i)", delim-start); + return NULL; + } + + CU_DEBUG("Escape is: %s", escape); + + if (STREQC(escape, "<")) + *result = '<'; + else if (STREQC(escape, ">")) + *result = '>'; + else if (STREQC(escape, """)) + *result = '\"'; + else + CU_DEBUG("Unhandled escape: `%s'", escape); + + len = strlen(escape); + + out: + free(escape); + + return (char *)start + len; +} + +static char *xml_decode(const char *input) +{ + const char *iptr = input; + char *optr; + char *res = NULL; + + res = malloc(strlen(input) + 1); + if (res == NULL) + return res; + + optr = res; + + while (iptr && (*iptr != '\0')) { + if (*iptr == '&') { + iptr = parse_esc(iptr, optr); + } else { + *optr = *iptr; + iptr++; + } + + optr++; + } + + return res; +} + +#endif + +int cu_parse_ei_xml(const CMPIBroker *broker, + const char *ns, + const char *xml, + CMPIInstance **instance) +{ + xmlDoc *doc = NULL; + xmlNode *root = NULL; + int ret = 0; + + doc = xmlReadMemory(xml, strlen(xml), NULL, NULL, 0); + if (doc == NULL) { + CU_DEBUG("Error reading decoded XML from memory"); + goto out; + } + + root = xmlDocGetRootElement(doc); + if (root == NULL) { + CU_DEBUG("Error getting root XML node"); + goto out; + } + + parse_instance(broker, ns, root, instance); + + out: + xmlFreeDoc(doc); + + return ret; +} + +/* + * Local Variables: + * mode: C + * c-set-style: "K&R" + * tab-width: 8 + * c-basic-offset: 8 + * indent-tabs-mode: nil + * End: + */ diff -r 03126c9af416 -r f6559a2f55ab eo_parser_xml.h --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/eo_parser_xml.h Fri Feb 08 15:17:35 2008 -0800 @@ -0,0 +1,40 @@ +/* + * Copyright IBM Corp. 2007 + * + * Authors: + * Dan Smith <danms@us.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 __EO_PARSER_XML_H +#define __EO_PARSER_XML_H + +int cu_parse_ei_xml(const CMPIBroker *broker, + const char *ns, + const char *xml, + CMPIInstance **instance); + +#endif + +/* + * Local Variables: + * mode: C + * c-set-style: "K&R" + * tab-width: 8 + * c-basic-offset: 8 + * indent-tabs-mode: nil + * End: + */
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1202512655 28800 # Node ID 7839b35e5c52454e71812f3ebb6e97bf07778423 # Parent f6559a2f55abab85baeabd56faaf7804c3ea30f3 [RFC][CU] Integrate the CIMXML EO parser with the libcmpiutil interface Signed-off-by: Dan Smith <danms@us.ibm.com> diff -r f6559a2f55ab -r 7839b35e5c52 eo_parser.c --- a/eo_parser.c Fri Feb 08 15:17:35 2008 -0800 +++ b/eo_parser.c Fri Feb 08 15:17:35 2008 -0800 @@ -31,13 +31,14 @@ #include "libcmpiutil.h" #include "eo_util_parser.h" +#include "eo_parser_xml.h" #define TEMPLATE "/tmp/tmp_eo_parse.XXXXXX" void eo_parse_restart(FILE *); int eo_parse_parseinstance(const CMPIBroker *, CMPIInstance **, const char *ns); -static int write_temp(char *eo) +static int write_temp(const char *eo) { int fd; int ret; @@ -62,13 +63,13 @@ static int write_temp(char *eo) return fd; } -int cu_parse_embedded_instance(const char *eo, - const CMPIBroker *broker, - const char *ns, - CMPIInstance **instance) +#ifdef HAVE_EOPARSER +static int parse_ei_mof(const CMPIBroker *broker, + const char *ns, + const char *eo, + CMPIInstance **instance) { int ret; -#ifdef HAVE_EOPARSER int fd; FILE *fp; @@ -81,10 +82,29 @@ int cu_parse_embedded_instance(const cha ret = eo_parse_parseinstance(broker, instance, ns); close(fd); + + return ret; +} +#endif + +int cu_parse_embedded_instance(const char *eo, + const CMPIBroker *broker, + const char *ns, + CMPIInstance **instance) +{ + if (strcasestr(eo, "<instance")) { + CU_DEBUG("Parsing CIMXML-style EI"); + return cu_parse_ei_xml(broker, ns, eo, instance); + } else { +#ifdef HAVE_EOPARSER + CU_DEBUG("Parsing MOF-style EI"); + return parse_ei_mof(broker, ns, eo, instance); #else - ret = 0; + CU_DEBUG("EI parser did not see `<instance', " + "assuming not CIMXML"); + return 0; #endif - return ret; + } } /*
On Fri, Feb 08, 2008 at 03:18:00PM -0800, Dan Smith wrote:
# HG changeset patch # User Dan Smith <danms@us.ibm.com> # Date 1202512655 28800 # Node ID f6559a2f55abab85baeabd56faaf7804c3ea30f3 # Parent 03126c9af41607dcaa8b197736dd5059c108261a [RFC][CU] Add EI CIMXML parser
This handles strings, bools, integers, and arrays of those. It doesn't yet handle nested objects, as we will need for RASD.HostResource[]. I need to do some further investigation of what the XML for that will look like.
I have tested this by taking an existing XML for DefineSystem() and changing the MemResourceAllocationSettingData field to this:
<INSTANCE CLASSNAME="Xen_MemResourceAllocationSettingData"> <PROPERTY NAME="CreationClassName" TYPE="string"> <VALUE>Xen_MemResourceAllocationSettingData</VALUE> </PROPERTY> <PROPERTY NAME="InstanceID" TYPE="string"> <VALUE>test/mem</VALUE> </PROPERTY> <PROPERTY NAME="ResourceType" TYPE="uint16"> <VALUE>4</VALUE> </PROPERTY> <PROPERTY NAME="VirtualQuantity" TYPE="uint64"> <VALUE>256 </VALUE> </PROPERTY> <PROPERTY.ARRAY NAME="Foo" TYPE="string"> <VALUE.ARRAY> <VALUE>Bar</VALUE> <VALUE>Baz</VALUE> </VALUE.ARRAY> </PROPERTY.ARRAY> </INSTANCE>
Oh the escaping fun :-) [...]
+static CMPIType get_node_value(const CMPIBroker *broker, + xmlNode *node, + const char *tstr, + CMPIValue *val) +{ + CMPIType type = CMPI_null; + char *content = NULL; + + if (node->type != XML_ELEMENT_NODE) { + CU_DEBUG("Non-element node: %s", node->name); + goto out; + } + + if (!STREQC((char *)node->name, "value")) { + CU_DEBUG("Expected <VALUE> but got <%s>", node->name); + goto out; + }
Assuming I understand the example, there is also no namespace in those element, checking node->ns == NULL might be a good idea (or not if namespaced)
+ content = get_content(node); + CU_DEBUG("Node content: %s", content); + type = get_property_value(broker, tstr, content, val); + free(content); + out: + return type; +} + +static CMPIType parse_array(const CMPIBroker *broker, + const char *tstr, + xmlNode *root, + CMPIArray **array) +{ + xmlNode *value; + CMPIValue *list = NULL; + int size = 0; + int cur = 0; + CMPIStatus s; + CMPIType type = CMPI_null; + int i; + + for (value = root->children; value; value = value->next) { + + if (value->type != XML_ELEMENT_NODE) + continue; + + if (cur == size) { + CMPIValue *tmp; + + size += 4; + tmp = realloc(list, sizeof(CMPIValue) * size);
when i have realloc in loop I usually do things like size *= 2 to escape a linear growing model. [..]
+ for (child = root->children; child; child = child->next) { + if (child->type == XML_ELEMENT_NODE){
check namespace here ?
+ if (STREQC((char *)child->name, "property")) + parse_property(broker, child, *inst); + CU_DEBUG("Escape is: %s", escape); + + if (STREQC(escape, "<")) + *result = '<'; + else if (STREQC(escape, ">")) + *result = '>'; + else if (STREQC(escape, """)) + *result = '\"';
Add & -> & and ' -> ''' would be good for completeness [...]
+int cu_parse_ei_xml(const CMPIBroker *broker, + const char *ns, + const char *xml, + CMPIInstance **instance) +{ + xmlDoc *doc = NULL; + xmlNode *root = NULL; + int ret = 0; + + doc = xmlReadMemory(xml, strlen(xml), NULL, NULL, 0);
I would add (XML_PARSE_NOENT | XML_PARSE_NOCDATA | XML_PARSE_NONET) to ask the parser to replace entities references, change CDATA section into normal text and forbid network access (should not occur, but better safe than sorry :-)
+ if (doc == NULL) { + CU_DEBUG("Error reading decoded XML from memory"); + goto out; + } + + root = xmlDocGetRootElement(doc); + if (root == NULL) { + CU_DEBUG("Error getting root XML node"); + goto out; + } +
Looks fine, maybe a small check that no memory allocation is lost is in order though, it's easy to forget something. Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
+static CMPIType parse_int_property(const char *string, + const char *type, + CMPIValue *val) +{ + int size; + bool sign; + CMPIType t; + int ret; + + if (sscanf(type, "uint%i", &size) == 1) + sign = false; + else if (sscanf(type, "int%i", &size) == 1) + sign = true; + else { + printf("Unknown integer type: `%s'", type); + return 0;
Should this be CMPI_null? I know they're equivalent, but it might be good to use CMPI_null for clarity (and the unlikely chance that the value for CMPI_null is changed)
+ +static bool parse_array_property(const CMPIBroker *broker, + xmlNode *node, + CMPIInstance *inst) +{ + char *name = NULL; + char *tstr = NULL; + bool ret = false; + xmlNode *val_arr; + CMPIArray *array; + CMPIType type; + + name = get_attr(node, "NAME"); + if (name == NULL) { + printf("Unnamed property\n"); + goto out; + } + + tstr = get_attr(node, "TYPE"); + if (tstr == NULL) { + printf("No type\n"); + goto out; + } + + printf("Array property `%s' of type `%s'\n", name, tstr); + + for (val_arr = node->children; val_arr; val_arr = val_arr->next) { + if (val_arr->type != XML_ELEMENT_NODE) + continue; + + if (!STREQC((char *)val_arr->name, "value.array")) { + printf("Expected <value.array> but got <%s>", + val_arr->name); + goto out; + } + + break; + }
Should we use some kind of found flag here? What if val_arr->type never equals XML_ELEMENT_NODE. Then we pass the last value of val_arr to the function below, even if the element type doesn't equal XML_ELEMENT_NODE. Or do we know for sure that val_arr will have an element with XML_ELEMENT_NODE type?
+ + type = parse_array(broker, tstr, val_arr, &array); + if (type != CMPI_null) { + CU_DEBUG("Setting array property"); + CMSetProperty(inst, name, &array, (CMPI_ARRAY | type)); + } + + out: + free(name); + free(tstr); + + return ret; +} +
+ +static CMPIStatus parse_instance(const CMPIBroker *broker, + const char *ns, + xmlNode *root, + CMPIInstance **inst) +{ + char *class = NULL; + xmlNode *child; + CMPIStatus s; + CMPIObjectPath *op; + + if (root->type != XML_ELEMENT_NODE) { + printf("First node is not <INSTANCE>\n"); + goto out; + } + + if (!STREQC((char *)root->name, "instance")) { + printf("Got node %s, expecting INSTANCE\n", root->name); + goto out; + } + + class = get_attr(root, "CLASSNAME"); + if (class == NULL) { + printf("No Classname\n"); + goto out; + }
For these statements above, should we set s.rc to some kind of error return code? Otherwise, we are returning a non-initialized status. Currently, this isn't a problem since the return value of parse_instance() isn't caught (see cu_parse_ei_xml() below)... so this might not need to be changed.
+ + CU_DEBUG("Instance of %s", class); + + op = CMNewObjectPath(broker, ns, class, &s); + if ((op == NULL) || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("Unable to create path for %s:%s", ns, class); + goto out; + } + + *inst = CMNewInstance(broker, op, &s); + if ((*inst == NULL) || (s.rc != CMPI_RC_OK)) { + CU_DEBUG("Unable to create inst for %s:%s", ns, class); + goto out; + } + + for (child = root->children; child; child = child->next) { + if (child->type == XML_ELEMENT_NODE){ + if (STREQC((char *)child->name, "property")) + parse_property(broker, child, *inst); + else if (STREQC((char *)child->name, "property.array")) + parse_array_property(broker, child, *inst); + else + printf("Unexpected node: %s\n", child->name); + } + } + + out: + free(class); + + return s; +} + +#if 0 + +/* This isn't currently used but I think I might need it for nested + * instances, depending on how they look. + */ + +static char *parse_esc(const char *start, char *result) +{ + char *delim; + char *escape = NULL; + int len = 1; + + delim = strchr(start, ';'); + if (delim == NULL) + goto out; + + escape = strndup(start, delim - start + 1); + if (escape == NULL) { + CU_DEBUG("Memory alloc failed (%i)", delim-start); + return NULL; + } + + CU_DEBUG("Escape is: %s", escape); + + if (STREQC(escape, "<")) + *result = '<'; + else if (STREQC(escape, ">")) + *result = '>'; + else if (STREQC(escape, """)) + *result = '\"'; + else + CU_DEBUG("Unhandled escape: `%s'", escape);
Would it be worthwhile to return NULL here? We return a pointer to a string, but the string might not be a valid element name because the escape isn't supported. Or should the caller check the result to make sure it's valid before continuing on?
+ + len = strlen(escape); + + out: + free(escape); + + return (char *)start + len; +} + +static char *xml_decode(const char *input) +{ + const char *iptr = input; + char *optr; + char *res = NULL; + + res = malloc(strlen(input) + 1); + if (res == NULL) + return res; + + optr = res; + + while (iptr && (*iptr != '\0')) {
Just a style note, but for non-boolean variables, aren't we trying to use something like: (iptr != NULL)?
+ if (*iptr == '&') { + iptr = parse_esc(iptr, optr); + } else { + *optr = *iptr; + iptr++; + } + + optr++; + } + + return res; +} + +#endif + +int cu_parse_ei_xml(const CMPIBroker *broker, + const char *ns, + const char *xml, + CMPIInstance **instance) +{ + xmlDoc *doc = NULL; + xmlNode *root = NULL; + int ret = 0; + + doc = xmlReadMemory(xml, strlen(xml), NULL, NULL, 0); + if (doc == NULL) { + CU_DEBUG("Error reading decoded XML from memory"); + goto out; ret isn't updated to indicate an error here. + } + + root = xmlDocGetRootElement(doc); + if (root == NULL) { + CU_DEBUG("Error getting root XML node"); + goto out; + } + + parse_instance(broker, ns, root, instance);
The return status isn't caught here. parse_instance() could return something other than CMPI_RC_OK if CMNewObjectPath() or CMNewInstance() fail.
+ + out: + xmlFreeDoc(doc); + + return ret; +} +
I know this just and RFC, but don't forget to remove printfs or convert them to CU_DEBUGs. =) -- Kaitlin Rupert IBM Linux Technology Center kaitlin@linux.vnet.ibm.com
KR> Should this be CMPI_null? I know they're equivalent, but it might be KR> good to use CMPI_null for clarity (and the unlikely chance that the KR> value for CMPI_null is changed) Yes, it should. I changed the signature of that function (about 1000 times) and forgot to update that return. KR> Should we use some kind of found flag here? What if val_arr->type KR> never equals XML_ELEMENT_NODE. Then we pass the last value of KR> val_arr to the function below, even if the element type doesn't KR> equal XML_ELEMENT_NODE. Technically the next call will fail appropriately, but yes, it should be handled better. KR> For these statements above, should we set s.rc to some kind of error KR> return code? Otherwise, we are returning a non-initialized status. Yes :) KR> Just a style note, but for non-boolean variables, aren't we trying to KR> use something like: (iptr != NULL)? Yes :) KR> The return status isn't caught here. parse_instance() could KR> return something other than CMPI_RC_OK if CMNewObjectPath() or KR> CMNewInstance() fail. Yep, that's broken indeed. KR> I know this just and RFC, but don't forget to remove printfs or KR> convert them to CU_DEBUGs. =) Gah! Every...Single...Time... :) Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
DV> Assuming I understand the example, there is also no namespace in DV> those element, checking node->ns == NULL might be a good idea (or DV> not if namespaced) Excuse my ignorance here, but why is this important? DV> when i have realloc in loop I usually do things like size *= 2 to DV> escape a linear growing model. Fair enough. DV> Add & -> & and ' -> ''' would be good for completeness Ah, yes, I actually meant to put something in my patch message asking what other escapes I should add. DV> I would add (XML_PARSE_NOENT | XML_PARSE_NOCDATA | DV> XML_PARSE_NONET) to ask the parser to replace entities references, DV> change CDATA section into normal text and forbid network access DV> (should not occur, but better safe than sorry :-) Okay. DV> Looks fine, maybe a small check that no memory allocation is lost is in DV> order though, it's easy to forget something. Yeah, I need to go back through with a fine-toothed comb. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
On Mon, Feb 11, 2008 at 10:41:02AM -0800, Dan Smith wrote:
DV> Assuming I understand the example, there is also no namespace in DV> those element, checking node->ns == NULL might be a good idea (or DV> not if namespaced)
Excuse my ignorance here, but why is this important?
To not misinterpret an element from a foreign namespace which could be used for something completeley different. Assuming your XML vocabulary is not defined to be in a namespace, just add the && (node->ns == NULL) to the test, it just help keeping the X of XML (eXtensible). It's cheap :-) Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
DV> To not misinterpret an element from a foreign namespace which DV> could be used for something completeley different. Assuming your DV> XML vocabulary is not defined to be in a namespace, just add the DV> && (node->ns == NULL) to the test, it just help keeping the X of DV> XML (eXtensible). It's cheap :-) Ah, okay, that makes sense. Thanks! -- Dan Smith IBM Linux Technology Center Open Hypervisor Team email: danms@us.ibm.com
participants (3)
-
Dan Smith -
Daniel Veillard -
Kaitlin Rupert