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. I considered just adding a ret = 0; near the bottom of virInterfaceDefParseBond, but see there is no cleanup in the error label. Instead, just return failure where failure occurs and return success if the end of the function is reached. --- src/conf/interface_conf.c | 56 +++++++++++++++++++++-------------------------- 1 file changed, 25 insertions(+), 31 deletions(-) diff --git a/src/conf/interface_conf.c b/src/conf/interface_conf.c index 9301ec0..3d45d5c 100644 --- a/src/conf/interface_conf.c +++ b/src/conf/interface_conf.c @@ -572,61 +572,58 @@ error: static int virInterfaceDefParseBond(virInterfaceDefPtr def, xmlXPathContextPtr ctxt) { - int ret = -1; + int res; unsigned long tmp; 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 = virXPathULong("string(./miimon/@freq)", ctxt, &tmp); + 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 = virXPathULong("string(./miimon/@downdelay)", ctxt, &tmp); + if (res == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("bond interface miimon downdelay invalid")); - goto error; - } else if (ret == 0) { + return -1; + } else if (res == 0) { def->data.bond.downdelay = (int) tmp; } - ret = virXPathULong("string(./miimon/@updelay)", ctxt, &tmp); - if (ret == -2) { + res = virXPathULong("string(./miimon/@updelay)", ctxt, &tmp); + if (res == -2) { virReportError(VIR_ERR_XML_ERROR, "%s", _("bond interface miimon updelay invalid")); - goto error; - } else if (ret == 0) { + return -1; + } else if (res == 0) { def->data.bond.updelay = (int) tmp; } 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 = virXPathULong("string(./arpmon/@interval)", ctxt, &tmp); + 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; @@ -635,18 +632,15 @@ virInterfaceDefParseBond(virInterfaceDefPtr def, 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 -- 1.8.0.1