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

[Richard W.M. Jones]

On Wed, Nov 16, 2016 at 12:59:38PM +0100, Pino Toscano wrote:
> 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);

To be consistent with the rest of the code, xpath_object_is_empty etc.

ACK with that changed.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

___
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs

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

[Pino Toscano]

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  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);
 
 /*  is the path/exportname.  Optional. */
@@ -583,13 +568,8 @@ for_each_disk (guestfs_h *g,
 /* .  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 =