On Fri, Oct 16, 2009 at 10:56:19AM -0400, laine(a)laine.org wrote:
static int
-virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceDefPtr def,
+virInterfaceDefParseProtoIPv4(virConnectPtr conn, virInterfaceProtocolDefPtr def,
xmlXPathContextPtr ctxt) {
- xmlNodePtr dhcp, ip;
- int ret = 0;
+ xmlNodePtr dhcp;
+ xmlNodePtr *ipNodes = NULL;
+ int nIpNodes, ii, ret = 0;
+ char *tmp;
+
+ tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
+ def->gateway = tmp;
dhcp = virXPathNode(conn, "./dhcp", ctxt);
if (dhcp != NULL)
ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
+ if (ret != 0)
+ return(ret);
+
+ nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
+ if (ipNodes == NULL)
+ return 0;
+
+ if (VIR_ALLOC_N(def->ips, nIpNodes) < 0) {
+ virReportOOMError(conn);
+ ret = -1;
+ goto error;
+ }
+
+ def->nips = 0;
+ for (ii = 0; ii < nIpNodes; ii++) {
+ virInterfaceIpDefPtr ip;
+
+ if (VIR_ALLOC(ip) < 0) {
+ virReportOOMError(conn);
+ break;
+ }
+
+ ctxt->node = ipNodes[ii];
+ ret = virInterfaceDefParseIp(conn, ip, ctxt);
+ if (ret != 0) {
+ virInterfaceIpDefFree(ip);
+ break;
+ }
+ def->ips[def->nips++] = ip;
+ }
+
+error:
+ VIR_FREE(ipNodes);
+ return(ret);
+}
I don't think the error reporting is quite right here. At the time of
declaration you initialize 'ret = 0', so those two error cases inside
the for loop which break out will end up returning 0. I think it is
better to initialize 'ret = -1', change the 'break' to 'goto
error',
and after the for loop put 'ret = 0' to mark success.
+
+static int
+virInterfaceDefParseProtoIPv6(virConnectPtr conn, virInterfaceProtocolDefPtr def,
+ xmlXPathContextPtr ctxt) {
+ xmlNodePtr dhcp, autoconf;
+ xmlNodePtr *ipNodes = NULL;
+ int nIpNodes, ii, ret = 0;
+ char *tmp;
+
+ tmp = virXPathString(conn, "string(./route[1]/@gateway)", ctxt);
+ def->gateway = tmp;
+
+ autoconf = virXPathNode(conn, "./autoconf", ctxt);
+ if (autoconf != NULL)
+ def->autoconf = 1;
+
+ dhcp = virXPathNode(conn, "./dhcp", ctxt);
+ if (dhcp != NULL)
+ ret = virInterfaceDefParseDhcp(conn, def, dhcp, ctxt);
if (ret != 0)
return(ret);
- ip = virXPathNode(conn, "./ip", ctxt);
- if (ip != NULL)
- ret = virInterfaceDefParseIp(conn, def, ip, ctxt);
+ nIpNodes = virXPathNodeSet(conn, "./ip", ctxt, &ipNodes);
+ if (ipNodes == NULL)
+ return 0;
+
+ if (VIR_ALLOC_N(def->ips, nIpNodes) < 0) {
+ virReportOOMError(conn);
+ ret = -1;
+ goto error;
+ }
+
+ def->nips = 0;
+ for (ii = 0; ii < nIpNodes; ii++) {
+
+ virInterfaceIpDefPtr ip;
+
+ if (VIR_ALLOC(ip) < 0) {
+ virReportOOMError(conn);
+ break;
+ }
+
+ ctxt->node = ipNodes[ii];
+ ret = virInterfaceDefParseIp(conn, ip, ctxt);
+ if (ret != 0) {
+ virInterfaceIpDefFree(ip);
+ break;
+ }
+ def->ips[def->nips++] = ip;
+ }
+
+error:
+ VIR_FREE(ipNodes);
return(ret);
}
Same here - that looks like it'll return 0 for several error
cases.
Regards,
Daniel
--
|: Red Hat, Engineering, London -o-
http://people.redhat.com/berrange/ :|
|:
http://libvirt.org -o-
http://virt-manager.org -o-
http://ovirt.org :|
|:
http://autobuild.org -o-
http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|