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

[Laine Stump]

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