[Libvirt-cim] [PATCH 2 of 3] [RFC][CU] Add EI CIMXML parser

[Daniel Veillard]

On Fri, Feb 08, 2008 at 03:18:00PM -0800, Dan Smith wrote:
> # HG changeset patch
> # User Dan Smith <danms at 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 at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/