Eric Blake wrote: > On 07/11/2014 01:04 PM, Jim Fehlig wrote: > >> David Kiarie wrote: >> >>> From: Kiarie Kahurani <davidkiarie4@xxxxxxxxx> >>> >>> Introduce the function >>> xenParseXMDisk(..........); >>> >>> Parses disk config >>> >>> signed-off-by: David Kiarie<davidkiarie4@xxxxxxxxx> >>> --- >>> 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). > Ah, should have read the source :). Thanks Eric. That explains my confusion as to why a crash was never reported in this code (or the vif parsing code). Regards, Jim -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list