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

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); }

2009/11/25 Jim Fehlig <jfehlig(a)novell.com&gt;: > 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; }