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 :-)