[libvirt] [PATCH 1/5] network_conf: use pointer to pointer type for srvrecords

Guannan Ren gren at redhat.com
Sun Jul 8 10:53:39 UTC 2012


Replace virNetworkDNSSrvRecordsDefPtr type with
virNetworkDNSSrvRecordsDefPtr * type for srvrecords field in
struct _virNetworkDNSDef.

Create two new helper functions to allocate and free memory of
type virNetworkDNSSrvRecordsDefPtr.
Fix a bug when any of values of port, priority, weight is not
given in SRV xml description, uninitialized of these three variables
are used  which causes the failure of creating dnsmasq subprocess.

before:
...
  <dns>
    <srv service='kerberos' protocol='tcp' target='pony.demo.redhat.com' port='88'/>
  </dns>
...

--srv-host=kerberos.tcp,pony.demo.redhat.com,88,1095468768,32544

after:
--srv-host=kerberos.tcp,pony.demo.redhat.com,88,,
---
 src/conf/network_conf.c     |  109 +++++++++++++++++++++++++++---------------
 src/conf/network_conf.h     |    5 ++-
 src/network/bridge_driver.c |   22 ++++----
 3 files changed, 85 insertions(+), 51 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index 515bc36..012be6a 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -140,10 +140,10 @@ static void virNetworkDNSDefFree(virNetworkDNSDefPtr def)
         VIR_FREE(def->hosts);
         if (def->nsrvrecords) {
             while (def->nsrvrecords--) {
-                VIR_FREE(def->srvrecords[def->nsrvrecords].domain);
-                VIR_FREE(def->srvrecords[def->nsrvrecords].service);
-                VIR_FREE(def->srvrecords[def->nsrvrecords].protocol);
-                VIR_FREE(def->srvrecords[def->nsrvrecords].target);
+                VIR_FREE(def->srvrecords[def->nsrvrecords]->domain);
+                VIR_FREE(def->srvrecords[def->nsrvrecords]->service);
+                VIR_FREE(def->srvrecords[def->nsrvrecords]->protocol);
+                VIR_FREE(def->srvrecords[def->nsrvrecords]->target);
             }
         }
         VIR_FREE(def->srvrecords);
@@ -569,6 +569,40 @@ error:
     return ret;
 }
 
+void
+virNetworkDNSSrvDefFree(virNetworkDNSSrvRecordsDefPtr srv)
+{
+    if (!srv)
+        return;
+
+    VIR_FREE(srv->domain);
+    VIR_FREE(srv->service);
+    VIR_FREE(srv->protocol);
+    VIR_FREE(srv->target);
+
+    VIR_FREE(srv);
+}
+
+virNetworkDNSSrvRecordsDefPtr
+virNetworkDNSSrvDefAlloc(void)
+{
+    virNetworkDNSSrvRecordsDefPtr srv = NULL;
+
+    if (VIR_ALLOC(srv) < 0) {
+        virReportOOMError();
+        return NULL;
+    }
+
+    /* The default for port is one */
+    srv->port = 1;
+
+    /* The defaults for weight and priority are zero */
+    srv->priority = 0;
+    srv->weight = 0;
+
+    return srv;
+}
+
 static int
 virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def,
                             xmlNodePtr cur,
@@ -583,6 +617,11 @@ virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def,
     int weight;
     int ret = 0;
 
+    virNetworkDNSSrvRecordsDefPtr srv = NULL;
+
+    if (!(srv = virNetworkDNSSrvDefAlloc()))
+        goto error;
+
     if (!(service = virXMLPropString(cur, "service"))) {
         virNetworkReportError(VIR_ERR_XML_DETAIL,
                               "%s", _("Missing required service attribute in dns srv record"));
@@ -614,37 +653,28 @@ virNetworkDNSSrvDefParseXML(virNetworkDNSDefPtr def,
         goto error;
     }
 
-    def->srvrecords[def->nsrvrecords].service = service;
-    def->srvrecords[def->nsrvrecords].protocol = protocol;
-    def->srvrecords[def->nsrvrecords].domain = NULL;
-    def->srvrecords[def->nsrvrecords].target = NULL;
-    def->srvrecords[def->nsrvrecords].port = 0;
-    def->srvrecords[def->nsrvrecords].priority = 0;
-    def->srvrecords[def->nsrvrecords].weight = 0;
+    srv->service = service;
+    srv->protocol = protocol;
 
     /* Following attributes are optional but we had to make sure they're NULL above */
     if ((target = virXMLPropString(cur, "target")) && (domain = virXMLPropString(cur, "domain"))) {
-        xmlNodePtr save_ctxt = ctxt->node;
+        srv->target = target;
+        srv->domain = domain;
 
+        xmlNodePtr save_ctxt = ctxt->node;
         ctxt->node = cur;
-        if (virXPathInt("string(./@port)", ctxt, &port))
-            def->srvrecords[def->nsrvrecords].port = port;
+        if (virXPathInt("string(./@port)", ctxt, &port) == 0)
+            srv->port = port;
 
-        if (virXPathInt("string(./@priority)", ctxt, &priority))
-            def->srvrecords[def->nsrvrecords].priority = priority;
+        if (virXPathInt("string(./@priority)", ctxt, &priority) == 0)
+            srv->priority = priority;
 
-        if (virXPathInt("string(./@weight)", ctxt, &weight))
-            def->srvrecords[def->nsrvrecords].weight = weight;
+        if (virXPathInt("string(./@weight)", ctxt, &weight) == 0)
+            srv->weight = weight;
         ctxt->node = save_ctxt;
-
-        def->srvrecords[def->nsrvrecords].domain = domain;
-        def->srvrecords[def->nsrvrecords].target = target;
-        def->srvrecords[def->nsrvrecords].port = port;
-        def->srvrecords[def->nsrvrecords].priority = priority;
-        def->srvrecords[def->nsrvrecords].weight = weight;
     }
 
-    def->nsrvrecords++;
+    def->srvrecords[def->nsrvrecords++] = srv;
 
     goto cleanup;
 
@@ -653,6 +683,7 @@ error:
     VIR_FREE(service);
     VIR_FREE(protocol);
     VIR_FREE(target);
+    virNetworkDNSSrvDefFree(srv);
 
     ret = -1;
 
@@ -1306,21 +1337,21 @@ virNetworkDNSDefFormat(virBufferPtr buf,
     }
 
     for (i = 0 ; i < def->nsrvrecords ; i++) {
-        if (def->srvrecords[i].service && def->srvrecords[i].protocol) {
+        if (def->srvrecords[i]->service && def->srvrecords[i]->protocol) {
             virBufferAsprintf(buf, "    <srv service='%s' protocol='%s'",
-                                  def->srvrecords[i].service,
-                                  def->srvrecords[i].protocol);
-
-            if (def->srvrecords[i].domain)
-                virBufferAsprintf(buf, " domain='%s'", def->srvrecords[i].domain);
-            if (def->srvrecords[i].target)
-                virBufferAsprintf(buf, " target='%s'", def->srvrecords[i].target);
-            if (def->srvrecords[i].port)
-                virBufferAsprintf(buf, " port='%d'", def->srvrecords[i].port);
-            if (def->srvrecords[i].priority)
-                virBufferAsprintf(buf, " priority='%d'", def->srvrecords[i].priority);
-            if (def->srvrecords[i].weight)
-                virBufferAsprintf(buf, " weight='%d'", def->srvrecords[i].weight);
+                                  def->srvrecords[i]->service,
+                                  def->srvrecords[i]->protocol);
+
+            if (def->srvrecords[i]->domain)
+                virBufferAsprintf(buf, " domain='%s'", def->srvrecords[i]->domain);
+            if (def->srvrecords[i]->target)
+                virBufferAsprintf(buf, " target='%s'", def->srvrecords[i]->target);
+            if (def->srvrecords[i]->port)
+                virBufferAsprintf(buf, " port='%d'", def->srvrecords[i]->port);
+            if (def->srvrecords[i]->priority)
+                virBufferAsprintf(buf, " priority='%d'", def->srvrecords[i]->priority);
+            if (def->srvrecords[i]->weight)
+                virBufferAsprintf(buf, " weight='%d'", def->srvrecords[i]->weight);
 
             virBufferAsprintf(buf, "/>\n");
         }
diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
index 4339a69..10b8328 100644
--- a/src/conf/network_conf.h
+++ b/src/conf/network_conf.h
@@ -83,6 +83,9 @@ struct _virNetworkDNSSrvRecordsDef {
     int weight;
 };
 
+void virNetworkDNSSrvDefFree(virNetworkDNSSrvRecordsDefPtr srv);
+virNetworkDNSSrvRecordsDefPtr virNetworkDNSSrvDefAlloc(void);
+
 struct _virNetworkDNSHostsDef {
     virSocketAddr ip;
     int nnames;
@@ -97,7 +100,7 @@ struct _virNetworkDNSDef {
     unsigned int nhosts;
     virNetworkDNSHostsDefPtr hosts;
     unsigned int nsrvrecords;
-    virNetworkDNSSrvRecordsDefPtr srvrecords;
+    virNetworkDNSSrvRecordsDefPtr *srvrecords;
 };
 
 typedef struct _virNetworkDNSDef *virNetworkDNSDefPtr;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 7e8de19..df999b9 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -526,31 +526,31 @@ networkBuildDnsmasqArgv(virNetworkObjPtr network,
         }
 
         for (i = 0; i < dns->nsrvrecords; i++) {
-            if (dns->srvrecords[i].service && dns->srvrecords[i].protocol) {
-                if (dns->srvrecords[i].port) {
-                    if (virAsprintf(&recordPort, "%d", dns->srvrecords[i].port) < 0) {
+            if (dns->srvrecords[i]->service && dns->srvrecords[i]->protocol) {
+                if (dns->srvrecords[i]->port) {
+                    if (virAsprintf(&recordPort, "%d", dns->srvrecords[i]->port) < 0) {
                         virReportOOMError();
                         goto cleanup;
                     }
                 }
-                if (dns->srvrecords[i].priority) {
-                    if (virAsprintf(&recordPriority, "%d", dns->srvrecords[i].priority) < 0) {
+                if (dns->srvrecords[i]->priority) {
+                    if (virAsprintf(&recordPriority, "%d", dns->srvrecords[i]->priority) < 0) {
                         virReportOOMError();
                         goto cleanup;
                     }
                 }
-                if (dns->srvrecords[i].weight) {
-                    if (virAsprintf(&recordWeight, "%d", dns->srvrecords[i].weight) < 0) {
+                if (dns->srvrecords[i]->weight) {
+                    if (virAsprintf(&recordWeight, "%d", dns->srvrecords[i]->weight) < 0) {
                         virReportOOMError();
                         goto cleanup;
                     }
                 }
 
                 if (virAsprintf(&record, "%s.%s.%s,%s,%s,%s,%s",
-                                dns->srvrecords[i].service,
-                                dns->srvrecords[i].protocol,
-                                dns->srvrecords[i].domain   ? dns->srvrecords[i].domain : "",
-                                dns->srvrecords[i].target   ? dns->srvrecords[i].target : "",
+                                dns->srvrecords[i]->service,
+                                dns->srvrecords[i]->protocol,
+                                dns->srvrecords[i]->domain   ? dns->srvrecords[i]->domain : "",
+                                dns->srvrecords[i]->target   ? dns->srvrecords[i]->target : "",
                                 recordPort                  ? recordPort                : "",
                                 recordPriority              ? recordPriority            : "",
                                 recordWeight                ? recordWeight              : "") < 0) {
-- 
1.7.7.5




More information about the libvir-list mailing list