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

Kaitlin Rupert kaitlin at linux.vnet.ibm.com
Mon Feb 11 16:44:10 UTC 2008


 >+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 at linux.vnet.ibm.com




More information about the Libvirt-cim mailing list