Re: [libvirt] [PREPOST 07/17] src/xenxs:Refactor disk config parsing code

On 07/11/2014 01:04 PM, Jim Fehlig wrote: > David Kiarie wrote: > >> From: Kiarie Kahurani <davidkiarie4(a)gmail.com&gt; >> >> Introduce the function >> xenParseXMDisk(..........); >> >> Parses disk config >> >> signed-off-by: David Kiarie&lt;davidkiarie4(a)gmail.com&gt; >> --- >> src/xenxs/xen_xm.c | 204 ++++++++++++++++++++++++++++------------------------- >> 1 file changed, 108 insertions(+), 96 deletions(-) >> >> >> @@ -912,7 +839,7 @@ xenParseXM(virConfPtr conf, int xendConfigVersion, >> >> /* Maintain list in sorted order according to target device name */ >> if (VIR_APPEND_ELEMENT(def->disks, def->ndisks, disk) < 0) >> - goto cleanup; >> + return -1; >> >> skipdisk: >> list = list->next; >> >> > The next line here is > > virDomainDiskDefFree(disk); > > which is not right in the case VIR_APPEND_ELEMENT succeeds (pre-existing > bug). 'disk' should be set to NULL if VIR_APPEND_ELEMENT succeeds, so > we don't free the disk we've just added to the disk list. > VIR_APPEND_ELEMENT(, , disk) sets disk=NULL on success, precisely so you don't have to worry about double-freeing it (if you call the macro, you are guaranteed either transfer semantics and success, or no change on failure).