[libvirt] [PATCH] fix xenDaemonListDefinedDomains
Daniel Veillard
veillard at redhat.com
Wed Nov 25 08:49:48 UTC 2009
On Wed, Nov 25, 2009 at 01:54:05AM +0100, Matthias Bolte wrote:
> 2009/11/25 Jim Fehlig <jfehlig at novell.com>:
> > 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 at veillard.com | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library http://libvirt.org/
More information about the libvir-list
mailing list