Move the checks for empty xmlXPathObjectPtr, and for extracting the result string out of it, to a new helper functions. This is just code motion, there should be no behaviour changes. --- src/libvirt-domain.c | 122 +++++++++++++++++++++------------------------------ 1 file changed, 50 insertions(+), 72 deletions(-) diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c index 4d4142d..baab307 100644 --- a/src/libvirt-domain.c +++ b/src/libvirt-domain.c @@ -43,6 +43,8 @@ static xmlDocPtr get_domain_xml (guestfs_h *g, virDomainPtr dom); static ssize_t for_each_disk (guestfs_h *g, virConnectPtr conn, xmlDocPtr doc, int (*f) (guestfs_h *g, const char *filename, const char *format, int readonly, const char *protocol, char *const *server, const char *username, void *data), void *data); static int libvirt_selinux_label (guestfs_h *g, xmlDocPtr doc, char **label_rtn, char **imagelabel_rtn); static char *filename_from_pool (guestfs_h *g, virConnectPtr conn, const char *pool_nane, const char *volume_name); +static bool xPathObjectIsEmpty (xmlXPathObjectPtr obj); +static char *xPathObjectGetString (xmlDocPtr doc, xmlXPathObjectPtr obj); static void ignore_errors (void *ignore, virErrorPtr ignore2) @@ -513,7 +515,6 @@ for_each_disk (guestfs_h *g, CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpusername = NULL; CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppool = NULL; CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xpvolume = NULL; - xmlAttrPtr attr; int readonly; int t; @@ -527,31 +528,21 @@ for_each_disk (guestfs_h *g, * Check the <disk type=..> attribute first to find out which one. */ xptype = xmlXPathEvalExpression (BAD_CAST "./@type", xpathCtx); - if (xptype == NULL || - xptype->nodesetval == NULL || - xptype->nodesetval->nodeNr == 0) { + if (xPathObjectIsEmpty (xptype)) continue; /* no type attribute, skip it */ - } - assert (xptype->nodesetval->nodeTab[0]); - assert (xptype->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE); - attr = (xmlAttrPtr) xptype->nodesetval->nodeTab[0]; - type = (char *) xmlNodeListGetString (doc, attr->children, 1); + type = xPathObjectGetString (doc, xptype); if (STREQ (type, "file")) { /* type = "file" so look at source/@file */ xpathCtx->node = nodes->nodeTab[i]; xpfilename = xmlXPathEvalExpression (BAD_CAST "./source/@file", xpathCtx); - if (xpfilename == NULL || - xpfilename->nodesetval == NULL || - xpfilename->nodesetval->nodeNr == 0) + if (xPathObjectIsEmpty (xpfilename)) continue; /* disk filename not found, skip this */ } else if (STREQ (type, "block")) { /* type = "block", use source/@dev */ xpathCtx->node = nodes->nodeTab[i]; xpfilename = xmlXPathEvalExpression (BAD_CAST "./source/@dev", xpathCtx); - if (xpfilename == NULL || - xpfilename->nodesetval == NULL || - xpfilename->nodesetval->nodeNr == 0) + if (xPathObjectIsEmpty (xpfilename)) continue; /* disk filename not found, skip this */ } else if (STREQ (type, "network")) { /* type = "network", use source/@name */ int hi; @@ -562,15 +553,9 @@ for_each_disk (guestfs_h *g, /* Get the protocol (e.g. "rbd"). Required. */ xpprotocol = xmlXPathEvalExpression (BAD_CAST "./source/@protocol", xpathCtx); - if (xpprotocol == NULL || - xpprotocol->nodesetval == NULL || - xpprotocol->nodesetval->nodeNr == 0) + if (xPathObjectIsEmpty (xpprotocol)) continue; - assert (xpprotocol->nodesetval->nodeTab[0]); - assert (xpprotocol->nodesetval->nodeTab[0]->type == - XML_ATTRIBUTE_NODE); - attr = (xmlAttrPtr) xpprotocol->nodesetval->nodeTab[0]; - protocol = (char *) xmlNodeListGetString (doc, attr->children, 1); + protocol = xPathObjectGetString (doc, xpprotocol); debug (g, "disk[%zu]: protocol: %s", i, protocol); /* <source name="..."> is the path/exportname. Optional. */ @@ -583,13 +568,8 @@ for_each_disk (guestfs_h *g, /* <auth username="...">. Optional. */ xpusername = xmlXPathEvalExpression (BAD_CAST "./auth/@username", xpathCtx); - if (xpusername != NULL && - xpusername->nodesetval != NULL && - xpusername->nodesetval->nodeNr != 0) { - assert (xpusername->nodesetval->nodeTab[0]); - assert (xpusername->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE); - attr = (xmlAttrPtr) xpusername->nodesetval->nodeTab[0]; - username = (char *) xmlNodeListGetString (doc, attr->children, 1); + if (!xPathObjectIsEmpty (xpusername)) { + username = xPathObjectGetString (doc, xpusername); debug (g, "disk[%zu]: username: %s", i, username); } @@ -641,28 +621,16 @@ for_each_disk (guestfs_h *g, /* Get the source pool. Required. */ xppool = xmlXPathEvalExpression (BAD_CAST "./source/@pool", xpathCtx); - if (xppool == NULL || - xppool->nodesetval == NULL || - xppool->nodesetval->nodeNr == 0) + if (xPathObjectIsEmpty (xppool)) continue; - assert (xppool->nodesetval->nodeTab[0]); - assert (xppool->nodesetval->nodeTab[0]->type == - XML_ATTRIBUTE_NODE); - attr = (xmlAttrPtr) xppool->nodesetval->nodeTab[0]; - pool = (char *) xmlNodeListGetString (doc, attr->children, 1); + pool = xPathObjectGetString (doc, xppool); /* Get the source volume. Required. */ xpvolume = xmlXPathEvalExpression (BAD_CAST "./source/@volume", xpathCtx); - if (xpvolume == NULL || - xpvolume->nodesetval == NULL || - xpvolume->nodesetval->nodeNr == 0) + if (xPathObjectIsEmpty (xpvolume)) continue; - assert (xpvolume->nodesetval->nodeTab[0]); - assert (xpvolume->nodesetval->nodeTab[0]->type == - XML_ATTRIBUTE_NODE); - attr = (xmlAttrPtr) xpvolume->nodesetval->nodeTab[0]; - volume = (char *) xmlNodeListGetString (doc, attr->children, 1); + volume = xPathObjectGetString (doc, xpvolume); debug (g, "disk[%zu]: pool: %s; volume: %s", i, pool, volume); @@ -679,13 +647,8 @@ for_each_disk (guestfs_h *g, if (filename == NULL) { assert (xpfilename); assert (xpfilename->nodesetval); - if (xpfilename->nodesetval->nodeNr > 0) { - assert (xpfilename->nodesetval->nodeTab[0]); - assert (xpfilename->nodesetval->nodeTab[0]->type == - XML_ATTRIBUTE_NODE); - attr = (xmlAttrPtr) xpfilename->nodesetval->nodeTab[0]; - filename = (char *) xmlNodeListGetString (doc, attr->children, 1); - } + if (xpfilename->nodesetval->nodeNr > 0) + filename = xPathObjectGetString (doc, xpfilename); else /* For network protocols (eg. nbd), name may be omitted. */ filename = safe_strdup (g, ""); @@ -696,22 +659,14 @@ for_each_disk (guestfs_h *g, /* Get the disk format (may not be set). */ xpathCtx->node = nodes->nodeTab[i]; xpformat = xmlXPathEvalExpression (BAD_CAST "./driver/@type", xpathCtx); - if (xpformat != NULL && - xpformat->nodesetval && - xpformat->nodesetval->nodeNr > 0) { - assert (xpformat->nodesetval->nodeTab[0]); - assert (xpformat->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE); - attr = (xmlAttrPtr) xpformat->nodesetval->nodeTab[0]; - format = (char *) xmlNodeListGetString (doc, attr->children, 1); - } + if (!xPathObjectIsEmpty (xpformat)) + format = xPathObjectGetString (doc, xpformat); /* Get the <readonly/> flag. */ xpathCtx->node = nodes->nodeTab[i]; xpreadonly = xmlXPathEvalExpression (BAD_CAST "./readonly", xpathCtx); readonly = 0; - if (xpreadonly != NULL && - xpreadonly->nodesetval && - xpreadonly->nodesetval->nodeNr > 0) + if (!xPathObjectIsEmpty (xpreadonly)) readonly = 1; if (f) @@ -774,23 +729,17 @@ connect_live (guestfs_h *g, virDomainPtr dom) if (nodes != NULL) { for (i = 0; i < nodes->nodeNr; ++i) { CLEANUP_XMLXPATHFREEOBJECT xmlXPathObjectPtr xppath = NULL; - xmlAttrPtr attr; /* See note in function above. */ xpathCtx->node = nodes->nodeTab[i]; /* The path is in <source path=..> attribute. */ xppath = xmlXPathEvalExpression (BAD_CAST "./source/@path", xpathCtx); - if (xppath == NULL || - xppath->nodesetval == NULL || - xppath->nodesetval->nodeNr == 0) { + if (xPathObjectIsEmpty (xppath)) { xmlXPathFreeObject (xppath); continue; /* no type attribute, skip it */ } - assert (xppath->nodesetval->nodeTab[0]); - assert (xppath->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE); - attr = (xmlAttrPtr) xppath->nodesetval->nodeTab[0]; - path = (char *) xmlNodeListGetString (doc, attr->children, 1); + path = xPathObjectGetString (doc, xppath); break; } } @@ -888,6 +837,35 @@ filename_from_pool (guestfs_h *g, virConnectPtr conn, return filename; } +/* Check that C<obj> is not empty. + */ +static bool +xPathObjectIsEmpty (xmlXPathObjectPtr obj) +{ + return obj == NULL || + obj->nodesetval == NULL || + obj->nodesetval->nodeNr == 0; +} + +/* Get the string value from C<obj>. + * + * C<obj> is I<required> to not be empty, i.e. that C<xPathObjectIsEmpty> + * is C<false>. + */ +static char * +xPathObjectGetString (xmlDocPtr doc, xmlXPathObjectPtr obj) +{ + xmlAttrPtr attr; + char *value; + + assert (obj->nodesetval->nodeTab[0]); + assert (obj->nodesetval->nodeTab[0]->type == XML_ATTRIBUTE_NODE); + attr = (xmlAttrPtr) obj->nodesetval->nodeTab[0]; + value = (char *) xmlNodeListGetString (doc, attr->children, 1); + + return value; +} + #else /* no libvirt at compile time */ #define NOT_IMPL(r) \ -- 2.7.4