[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] conf: eliminate redundat VIR_ALLOC of first element of hosts.



On 08/11/2011 02:28 AM, Daniel Veillard wrote:
On Thu, Aug 11, 2011 at 12:15:03AM -0400, Laine Stump wrote:
virNetworkDNSHostsDefParseXML was calling VIR_ALLOC(def->hosts) if
def->nhosts was 0. This is a waste of time, though, since
VIR_REALLOC_N is called a few lines further down, prior to any use of
def->hosts.
---
  src/conf/network_conf.c |    8 --------
  1 files changed, 0 insertions(+), 8 deletions(-)

diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index e055094..109739f 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -495,14 +495,6 @@ virNetworkDNSHostsDefParseXML(virNetworkDNSDefPtr def,
      virSocketAddr inaddr;
      int ret = -1;

-    if (def->hosts == NULL) {
-        if (VIR_ALLOC(def->hosts)<  0) {
-            virReportOOMError();
-            goto error;
-        }
-        def->nhosts = 0;
-    }
-
      if (!(ip = virXMLPropString(node, "ip")) ||
          (virSocketParseAddr(ip,&inaddr, AF_UNSPEC)<  0)) {
          virNetworkReportError(VIR_ERR_XML_DETAIL,
   But that allowed to make sure that def->nhosts was initialized in that
case and that's used later. Maybe just remove the
   if (VIR_ALLOC(def->hosts)<  0) {
   ...
   }

block ?

That crossed my mind when I first thought of cutting out this code, so I audited just to be sure - virNetworkDNSHostsDefParseXML() is called only by virNetworkDNSDefParseXML(), which always allocates the passed in def with VIR_ALLOC(), so def->nhosts is guaranteed to be initialized to 0. (Every other "nItems" in the virNetworkDNSDef is implicitly initialized; this is the only odd one, so removing it will eliminate potential confusion by anyone who is "coding by example". In general it seems that all of the ...ParseXML() functions assume that they're working with a struct that has been 0-filled.)

If we don't want to assume 0-filled structs, there's a *lot* of work to be done in the conf directory :-)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]