2:34 p.m.
Making this change makes it easier to spot the memory leaks
that will be fixed in the next patch.
* cfg.mk (sc_prohibit_xmlGetProp): New rule.
* .x-sc_prohibit_xmlGetProp: New exception.
* Makefile.am (EXTRA_DIST): Ship exception file.
* tools/virsh.c (cmdDetachInterface, cmdDetachDisk): Adjust
offenders.
* src/conf/storage_conf.c (virStoragePoolDefParseSource):
Likewise.
* src/conf/network_conf.c (virNetworkDHCPRangeDefParseXML)
(virNetworkIPParseXML): Likewise.
---
valgrind picked up a memory leak in virNetworkDHCPRangeDefParseXML,
but our use of non-idiomatic string management made it harder to
see at first glance. This makes the few outliers consistent with
all the rest of the code base.
.x-sc_prohibit_xmlGetProp | 1 +
Makefile.am | 1 +
cfg.mk | 6 +++++
src/conf/network_conf.c | 56 ++++++++++++++++++++++----------------------
src/conf/storage_conf.c | 4 +-
tools/virsh.c | 20 ++++++++--------
6 files changed, 48 insertions(+), 40 deletions(-)
create mode 100644 .x-sc_prohibit_xmlGetProp
diff --git a/.x-sc_prohibit_xmlGetProp b/.x-sc_prohibit_xmlGetProp
new file mode 100644
index 0000000..f6d7ee2
--- /dev/null
+++ b/.x-sc_prohibit_xmlGetProp
@@ -0,0 +1 @@
+^src/util/xml.c$
diff --git a/Makefile.am b/Makefile.am
index d3f8876..e4d369d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -39,6 +39,7 @@ EXTRA_DIST = \
.x-sc_prohibit_strncpy \
.x-sc_prohibit_test_minus_ao \
.x-sc_prohibit_VIR_ERR_NO_MEMORY \
+ .x-sc_prohibit_xmlGetProp \
.x-sc_require_config_h \
.x-sc_require_config_h_first \
.x-sc_trailing_blank \
diff --git a/cfg.mk b/cfg.mk
index 0851f44..4c5afee 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -326,6 +326,12 @@ sc_prohibit_gethostby:
halt='use getaddrinfo, not gethostby*' \
$(_sc_search_regexp)
+# raw xmlGetProp requires some nasty casts
+sc_prohibit_xmlGetProp:
+ @prohibit='\<xmlGetProp *\(' \
+ halt='use virXMLPropString, not xmlGetProp' \
+ $(_sc_search_regexp)
+
# Many of the function names below came from this filter:
# git grep -B2 '\<_('|grep -E '\.c- *[[:alpha:]_][[:alnum:]_]*
?\(.*[,;]$' \
# |sed 's/.*\.c- *//'|perl -pe 's/ ?\(.*//'|sort -u \
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 3f2bf1f..9868250 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -222,24 +222,24 @@ virNetworkDHCPRangeDefParseXML(virNetworkDefPtr def,
virSocketAddr saddr, eaddr;
int range;
- if (!(start = (char *) xmlGetProp(cur, BAD_CAST "start"))) {
+ if (!(start = virXMLPropString(cur, "start"))) {
cur = cur->next;
continue;
}
- if (!(end = (char *) xmlGetProp(cur, BAD_CAST "end"))) {
- xmlFree(start);
+ if (!(end = virXMLPropString(cur, "end"))) {
+ VIR_FREE(start);
cur = cur->next;
continue;
}
if (virSocketParseAddr(start, &saddr, AF_UNSPEC) < 0) {
- xmlFree(start);
- xmlFree(end);
+ VIR_FREE(start);
+ VIR_FREE(end);
return -1;
}
if (virSocketParseAddr(end, &eaddr, AF_UNSPEC) < 0) {
- xmlFree(start);
- xmlFree(end);
+ VIR_FREE(start);
+ VIR_FREE(end);
return -1;
}
@@ -248,14 +248,14 @@ virNetworkDHCPRangeDefParseXML(virNetworkDefPtr def,
virNetworkReportError(VIR_ERR_XML_ERROR,
_("dhcp range '%s' to '%s'
invalid"),
start, end);
- xmlFree(start);
- xmlFree(end);
+ VIR_FREE(start);
+ VIR_FREE(end);
return -1;
}
if (VIR_REALLOC_N(def->ranges, def->nranges + 1) < 0) {
- xmlFree(start);
- xmlFree(end);
+ VIR_FREE(start);
+ VIR_FREE(end);
virReportOOMError();
return -1;
}
@@ -264,19 +264,19 @@ virNetworkDHCPRangeDefParseXML(virNetworkDefPtr def,
def->nranges++;
} else if (cur->type == XML_ELEMENT_NODE &&
xmlStrEqual(cur->name, BAD_CAST "host")) {
- xmlChar *mac, *name, *ip;
+ char *mac, *name, *ip;
unsigned char addr[6];
virSocketAddr inaddr;
- mac = xmlGetProp(cur, BAD_CAST "mac");
+ mac = virXMLPropString(cur, "mac");
if ((mac != NULL) &&
- (virParseMacAddr((const char *) mac, &addr[0]) != 0)) {
+ (virParseMacAddr(mac, &addr[0]) != 0)) {
virNetworkReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse MAC address
'%s'"),
mac);
VIR_FREE(mac);
}
- name = xmlGetProp(cur, BAD_CAST "name");
+ name = virXMLPropString(cur, "name");
if ((name != NULL) && (!c_isalpha(name[0]))) {
virNetworkReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot use name address
'%s'"),
@@ -292,8 +292,8 @@ virNetworkDHCPRangeDefParseXML(virNetworkDefPtr def,
cur = cur->next;
continue;
}
- ip = xmlGetProp(cur, BAD_CAST "ip");
- if (virSocketParseAddr((const char *)ip, &inaddr, AF_UNSPEC) < 0) {
+ ip = virXMLPropString(cur, "ip");
+ if (virSocketParseAddr(ip, &inaddr, AF_UNSPEC) < 0) {
VIR_FREE(ip);
VIR_FREE(mac);
VIR_FREE(name);
@@ -307,29 +307,29 @@ virNetworkDHCPRangeDefParseXML(virNetworkDefPtr def,
virReportOOMError();
return -1;
}
- def->hosts[def->nhosts].mac = (char *)mac;
- def->hosts[def->nhosts].name = (char *)name;
+ def->hosts[def->nhosts].mac = mac;
+ def->hosts[def->nhosts].name = name;
def->hosts[def->nhosts].ip = inaddr;
def->nhosts++;
} else if (cur->type == XML_ELEMENT_NODE &&
xmlStrEqual(cur->name, BAD_CAST "bootp")) {
- xmlChar *file;
- xmlChar *server;
+ char *file;
+ char *server;
virSocketAddr inaddr;
memset(&inaddr, 0, sizeof(inaddr));
- if (!(file = xmlGetProp(cur, BAD_CAST "file"))) {
+ if (!(file = virXMLPropString(cur, "file"))) {
cur = cur->next;
continue;
}
- server = xmlGetProp(cur, BAD_CAST "server");
+ server = virXMLPropString(cur, "server");
if (server &&
- virSocketParseAddr((const char *)server, &inaddr, AF_UNSPEC) < 0)
+ virSocketParseAddr(server, &inaddr, AF_UNSPEC) < 0)
return -1;
- def->bootfile = (char *)file;
+ def->bootfile = file;
def->bootserver = inaddr;
}
@@ -354,14 +354,14 @@ virNetworkIPParseXML(virNetworkDefPtr def,
} else if (cur->type == XML_ELEMENT_NODE &&
xmlStrEqual(cur->name, BAD_CAST "tftp")) {
- xmlChar *root;
+ char *root;
- if (!(root = xmlGetProp(cur, BAD_CAST "root"))) {
+ if (!(root = virXMLPropString(cur, "root"))) {
cur = cur->next;
continue;
}
- def->tftproot = (char *)root;
+ def->tftproot = root;
}
cur = cur->next;
diff --git a/src/conf/storage_conf.c b/src/conf/storage_conf.c
index 45285de..f7f471e 100644
--- a/src/conf/storage_conf.c
+++ b/src/conf/storage_conf.c
@@ -446,14 +446,14 @@ virStoragePoolDefParseSource(xmlXPathContextPtr ctxt,
}
for (i = 0 ; i < nsource ; i++) {
- xmlChar *path = xmlGetProp(nodeset[i], BAD_CAST "path");
+ char *path = virXMLPropString(nodeset[i], "path");
if (path == NULL) {
VIR_FREE(nodeset);
virStorageReportError(VIR_ERR_XML_ERROR,
"%s", _("missing storage pool source device
path"));
goto cleanup;
}
- source->devices[i].path = (char *)path;
+ source->devices[i].path = path;
}
source->ndevice = nsource;
}
diff --git a/tools/virsh.c b/tools/virsh.c
index 0ea1930..aec7749 100644
--- a/tools/virsh.c
+++ b/tools/virsh.c
@@ -8434,7 +8434,6 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
xmlXPathObjectPtr obj=NULL;
xmlXPathContextPtr ctxt = NULL;
xmlNodePtr cur = NULL;
- xmlChar *tmp_mac = NULL;
xmlBufferPtr xml_buf = NULL;
char *doc, *mac =NULL, *type;
char buf[64];
@@ -8485,10 +8484,11 @@ cmdDetachInterface(vshControl *ctl, const vshCmd *cmd)
for (; i < obj->nodesetval->nodeNr; i++) {
cur = obj->nodesetval->nodeTab[i]->children;
while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name,
BAD_CAST "mac")) {
- tmp_mac = xmlGetProp(cur, BAD_CAST "address");
- diff_mac = virMacAddrCompare ((char *) tmp_mac, mac);
- xmlFree(tmp_mac);
+ if (cur->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(cur->name, BAD_CAST "mac")) {
+ char *tmp_mac = virXMLPropString(cur, "address");
+ diff_mac = virMacAddrCompare (tmp_mac, mac);
+ VIR_FREE(tmp_mac);
if (!diff_mac) {
goto hit;
}
@@ -8689,7 +8689,6 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
xmlXPathObjectPtr obj=NULL;
xmlXPathContextPtr ctxt = NULL;
xmlNodePtr cur = NULL;
- xmlChar *tmp_tgt = NULL;
xmlBufferPtr xml_buf = NULL;
virDomainPtr dom = NULL;
char *doc, *target;
@@ -8734,10 +8733,11 @@ cmdDetachDisk(vshControl *ctl, const vshCmd *cmd)
for (; i < obj->nodesetval->nodeNr; i++) {
cur = obj->nodesetval->nodeTab[i]->children;
while (cur != NULL) {
- if (cur->type == XML_ELEMENT_NODE && xmlStrEqual(cur->name,
BAD_CAST "target")) {
- tmp_tgt = xmlGetProp(cur, BAD_CAST "dev");
- diff_tgt = xmlStrEqual(tmp_tgt, BAD_CAST target);
- xmlFree(tmp_tgt);
+ if (cur->type == XML_ELEMENT_NODE &&
+ xmlStrEqual(cur->name, BAD_CAST "target")) {
+ char *tmp_tgt = virXMLPropString(cur, "dev");
+ diff_tgt = STREQ(tmp_tgt, target);
+ VIR_FREE(tmp_tgt);
if (diff_tgt) {
goto hit;
}
--
1.7.3.2