Fix parsing of bond interface XML

Noticed that parsing bond interface XML containing the miimon element
fails

  <interface type="bond" name="bond0">
    ...
    <bond mode="active-backup">
      <miimon freq="100" carrier="netif"/>
      ...
    </bond>
  </interface>

This configuration does not contain the optional updelay and downdelay
attributes, but parsing will fail due to returning the result of
virXPathULong (a -1 when the attribute doesn't exist) from
virInterfaceDefParseBond after examining the updelay attribute.

While fixing this bug, cleanup the function to use virXPathInt instead
of virXPathULong, and store the result directly instead of using a tmp
variable.  Using virXPathInt actually fixes a potential silent
truncation bug noted by Eric Blake.

Also, there is no cleanup in the error label.  Remove the label,
returning failure where failure occurs and success if the end of the
function is reached.

diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c
index 9301ec0f61386614fab2abeb41ba1f35ac6c5c7c..7ca9c8618c277417c4b301eb9b68cee7763bc8ce 100644 (file)
--- a/src/conf/interface_conf.c
+++ b/src/conf/interface_conf.c
@@ -572,81 +572,72 @@ error:
 static int
 virInterfaceDefParseBond(virInterfaceDefPtr def,
                          xmlXPathContextPtr ctxt) {
-    int ret = -1;
-    unsigned long tmp;
+    int res;
 
     def->data.bond.mode = virInterfaceDefParseBondMode(ctxt);
     if (def->data.bond.mode < 0)
-        goto error;
+        return -1;
 
-    ret = virInterfaceDefParseBondItfs(def, ctxt);
-    if (ret != 0)
-       goto error;
+    if (virInterfaceDefParseBondItfs(def, ctxt) != 0)
+        return -1;
 
     if (virXPathNode("./miimon[1]", ctxt) != NULL) {
         def->data.bond.monit = VIR_INTERFACE_BOND_MONIT_MII;
 
-        ret = virXPathULong("string(./miimon/@freq)", ctxt, &tmp);
-        if ((ret == -2) || (ret == -1)) {
+        res = virXPathInt("string(./miimon/@freq)", ctxt,
+                          &def->data.bond.frequency);
+        if ((res == -2) || (res == -1)) {
             virReportError(VIR_ERR_XML_ERROR,
                            "%s", _("bond interface miimon freq missing or invalid"));
-            goto error;
+            return -1;
         }
-        def->data.bond.frequency = (int) tmp;
 
-        ret = virXPathULong("string(./miimon/@downdelay)", ctxt, &tmp);
-        if (ret == -2) {
+        res = virXPathInt("string(./miimon/@downdelay)", ctxt,
+                          &def->data.bond.downdelay);
+        if (res == -2) {
             virReportError(VIR_ERR_XML_ERROR,
                            "%s", _("bond interface miimon downdelay invalid"));
-            goto error;
-        } else if (ret == 0) {
-            def->data.bond.downdelay = (int) tmp;
+            return -1;
         }
 
-        ret = virXPathULong("string(./miimon/@updelay)", ctxt, &tmp);
-        if (ret == -2) {
+        res = virXPathInt("string(./miimon/@updelay)", ctxt,
+                          &def->data.bond.updelay);
+        if (res == -2) {
             virReportError(VIR_ERR_XML_ERROR,
                            "%s", _("bond interface miimon updelay invalid"));
-            goto error;
-        } else if (ret == 0) {
-            def->data.bond.updelay = (int) tmp;
+            return -1;
         }
 
         def->data.bond.carrier = virInterfaceDefParseBondMiiCarrier(ctxt);
-        if (def->data.bond.carrier < 0) {
-            ret = -1;
-            goto error;
-        }
+        if (def->data.bond.carrier < 0)
+            return -1;
 
     } else if (virXPathNode("./arpmon[1]", ctxt) != NULL) {
 
         def->data.bond.monit = VIR_INTERFACE_BOND_MONIT_ARP;
 
-        ret = virXPathULong("string(./arpmon/@interval)", ctxt, &tmp);
-        if ((ret == -2) || (ret == -1)) {
+        res = virXPathInt("string(./arpmon/@interval)", ctxt,
+                          &def->data.bond.interval);
+        if ((res == -2) || (res == -1)) {
             virReportError(VIR_ERR_XML_ERROR,
                            "%s", _("bond interface arpmon interval missing or invalid"));
-            goto error;
+            return -1;
         }
-        def->data.bond.interval = (int) tmp;
 
         def->data.bond.target =
             virXPathString("string(./arpmon/@target)", ctxt);
         if (def->data.bond.target == NULL) {
             virReportError(VIR_ERR_XML_ERROR,
                            "%s", _("bond interface arpmon target missing"));
-            ret = -1;
-            goto error;
+            return -1;
         }
 
         def->data.bond.validate = virInterfaceDefParseBondArpValid(ctxt);
-        if (def->data.bond.validate < 0) {
-            ret = -1;
-            goto error;
-        }
+        if (def->data.bond.validate < 0)
+            return -1;
     }
-error:
-    return ret;
+
+    return 0;
 }
 
 static int