Re: [libvirt] [PATCH 04/18] domain_conf: cleanup virDomainGraphicsListensParseXML

[Ján Tomko]

On Mon, Apr 04, 2016 at 03:20:21PM +0200, Pavel Hrdina wrote:
> Refactor the listen parser to use only one loop.
> 
> Signed-off-by: Pavel Hrdina 
> ---
>  src/conf/domain_conf.c | 117 
> +
>  1 file changed, 50 insertions(+), 67 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index a0ef3d9..9d48d07 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c

>  
> +/* listen attribute of  is also supported by these,
> + * but must match the 'address' attribute of the first listen
> + * that is type='address' (if present) */
> +listenAddr = virXMLPropString(node, "listen");
> +if (listenAddr && !listenAddr[0])
> +VIR_FREE(listenAddr);
> +
> +if (STREQ_NULLABLE(address->address, listenAddr) < 0) {

This is dead code, since it expands to strcmp(a,b) == 0.

Drop the comparison against zero and use STRNEQ_NULLABLE instead.

Also, the check should only be done if address is non-NULL, for XMLs
with no  elements.

ACK with that fixed.

Jan

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 04/18] domain_conf: cleanup virDomainGraphicsListensParseXML

[Pavel Hrdina]

Refactor the listen parser to use only one loop.

Signed-off-by: Pavel Hrdina 
---
 src/conf/domain_conf.c | 117 +
 1 file changed, 50 insertions(+), 67 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index a0ef3d9..9d48d07 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -10681,86 +10681,69 @@ 
virDomainGraphicsListensParseXML(virDomainGraphicsDefPtr def,
  xmlXPathContextPtr ctxt,
  unsigned int flags)
 {
-int nListens;
 xmlNodePtr *listenNodes = NULL;
-char *listenAddr = NULL;
 xmlNodePtr save = ctxt->node;
+virDomainGraphicsListenDefPtr address = NULL;
+char *listenAddr = NULL;
+int nListens;
 int ret = -1;
 
 ctxt->node = node;
-if (def->type == VIR_DOMAIN_GRAPHICS_TYPE_VNC ||
-def->type == VIR_DOMAIN_GRAPHICS_TYPE_RDP ||
-def->type == VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
 
-/* parse the  subelements for graphics types that support it */
-nListens = virXPathNodeSet("./listen", ctxt, );
-if (nListens < 0)
-goto error;
+if (def->type != VIR_DOMAIN_GRAPHICS_TYPE_VNC &&
+def->type != VIR_DOMAIN_GRAPHICS_TYPE_RDP &&
+def->type != VIR_DOMAIN_GRAPHICS_TYPE_SPICE) {
+ret = 0;
+goto error;
+}
 
-if (nListens > 0) {
-size_t i;
+/* parse the  subelements for graphics types that support it */
+nListens = virXPathNodeSet("./listen", ctxt, );
+if (nListens < 0)
+goto error;
+
+if (nListens > 0) {
+size_t i;
 
-if (VIR_ALLOC_N(def->listens, nListens) < 0)
+if (VIR_ALLOC_N(def->listens, nListens) < 0)
+goto error;
+
+for (i = 0; i < nListens; i++) {
+if (virDomainGraphicsListenDefParseXML(>listens[i],
+   listenNodes[i],
+   flags) < 0)
 goto error;
 
-for (i = 0; i < nListens; i++) {
-int rv = virDomainGraphicsListenDefParseXML(>listens[i],
-listenNodes[i],
-flags);
-if (rv < 0)
-goto error;
-def->nListens++;
-}
-VIR_FREE(listenNodes);
-}
-
-/* listen attribute of  is also supported by these,
- * but must match the 'address' attribute of the first listen
- * that is type='address' (if present) */
-listenAddr = virXMLPropString(node, "listen");
-if (listenAddr && !listenAddr[0])
-VIR_FREE(listenAddr);
-
-if (listenAddr) {
-if (def->nListens == 0) {
-/* There were no  elements, so we can just
- * directly set listenAddr as listens[0]->address */
-if (virDomainGraphicsListenSetAddress(def, 0, listenAddr,
-  -1, true) < 0)
-goto error;
-} else {
-/* There is at least 1 listen element, so we look for
- * the first listen of type='address', and make sure
- * its address matches the listen attribute from
- * graphics. */
-bool matched = false;
-const char *found = NULL;
-size_t i;
-
-for (i = 0; i < nListens; i++) {
-if (virDomainGraphicsListenGetType(def, i)
-== VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS) {
-found = virDomainGraphicsListenGetAddress(def, i);
-if (STREQ_NULLABLE(found, listenAddr))
-matched = true;
-break;
-}
-}
-if (found && !matched) {
-virReportError(VIR_ERR_XML_ERROR,
-   _("graphics listen attribute %s must match 
address "
- "attribute of first listen element (found 
%s)"),
-   listenAddr, found);
-goto error;
-} else if (!found) {
-/* quietly ignore listen address if none of the listens
- * are of type address */
-VIR_FREE(listenAddr);
-}
-}
+if (!address &&
+def->listens[i].type == 
VIR_DOMAIN_GRAPHICS_LISTEN_TYPE_ADDRESS)
+address = >listens[i];
+
+def->nListens++;
 }
+VIR_FREE(listenNodes);
 }
 
+/* listen attribute of  is also supported by these,
+ * but must match the 'address' attribute of the first listen
+ *