On Wed, Nov 25, 2009 at 01:54:05AM +0100, Matthias Bolte wrote: > 2009/11/25 Jim Fehlig <jfehlig@xxxxxxxxxx>: > > Commit 790f0b3057787bb64da8c46c111ff8d3eff7b2af causes contents of names > > array to be freed even on success, resulting in no listing of defined > > but inactive Xen domains. Patch below fixes it. > > > > Regards, > > Jim > > > > Good catch, I just reviewed this commit to see if I've caused similar > bugs elsewhere, but this seems to be the only one. > > > Index: libvirt-0.7.4/src/xen/xend_internal.c > > =================================================================== > > --- libvirt-0.7.4.orig/src/xen/xend_internal.c > > +++ libvirt-0.7.4/src/xen/xend_internal.c > > @@ -4693,13 +4693,14 @@ xenDaemonListDefinedDomains(virConnectPt > > } > > > > if (ret >= maxnames) > > - break; > > + goto out; > > } > > > > error: > > for (i = 0; i < ret; ++i) > > VIR_FREE(names[i]); > > > > +out: > > sexpr_free(root); > > return(ret); > > } > > Your patch doesn't fix the problem in all situations. If maxnames is > larger than the actual number of domains then goto out is never > executed. > > I also forgot to set ret to -1 after freeing the names, this needs to > be fixed too. > > I propose the attached patch to solve this issues. > > Matthias > diff --git a/src/xen/xend_internal.c b/src/xen/xend_internal.c > index d61e9e6..a0c7d77 100644 > --- a/src/xen/xend_internal.c > +++ b/src/xen/xend_internal.c > @@ -4696,12 +4696,17 @@ xenDaemonListDefinedDomains(virConnectPtr conn, char **const names, int maxnames > break; > } > > +cleanup: > + sexpr_free(root); > + return(ret); > + > error: > for (i = 0; i < ret; ++i) > VIR_FREE(names[i]); > > - sexpr_free(root); > - return(ret); > + ret = -1; > + > + goto cleanup; > } > ACK, that looks right to me ! Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@xxxxxxxxxxxx | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/ -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list