[Libguestfs] [PATCH 1/2] libvirt: un-duplicate XPath code

16 Nov 2016

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

    

[Libguestfs] [PATCH 1/2] libvirt: un-duplicate XPath code

Pino Toscano