maint: prohibit most uses of xmlGetProp

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.

diff --git a/.x-sc_prohibit_xmlGetProp b/.x-sc_prohibit_xmlGetProp
new file mode 100644 (file)
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 bd70a3a8ab3ca9dd57ccca99b913d1bf26d6db0e..c525e65bd8f3592693926a19b82490592b03015f 100644 (file)
--- a/Makefile.am
+++ b/Makefile.am
@@ -38,6 +38,7 @@ syntax_check_exceptions = \
   .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 5b4a603d7c73646166be65136a09be66c597105f..ee182b7cf2f86989a59a5c7827deb79af0dab30d 100644 (file)
--- a/cfg.mk
+++ b/cfg.mk
@@ -335,6 +335,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 3f2bf1f5ed7457e89bb9acc4ab576478225f2dfb..9868250108c90fc8cbd6eebbbfd23da3dd75a549 100644 (file)
--- 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 45285de9c00f8945a69cceb5fb70f2f09946ba61..f7f471e630874b0bbdf234541d6ebe1f0064af3f 100644 (file)
--- 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 0ea1930e4b0c494d8fe556f8ac4c954e2da62416..aec7749e0945bed0754240015ed0a0b160791c2b 100644 (file)
--- 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;
                 }