On Mon, Sep 07, 2009 at 05:41:18PM +0200, Jim Meyering wrote:
> This one took a little thinking.
>
> I'm not 100% sure that the comment in my log is sufficient
> justification for removing the virGetDomain call. If we leave it,
> we'll have to free it and remove the XXX comment.
>
> >From 324e07f17e6a2ed6df236af955adc693670d1b16 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering@xxxxxxxxxx>
> Date: Fri, 4 Sep 2009 19:46:59 +0200
> Subject: [PATCH] xm_internal.c: remove dead store and associated leak
>
> * src/xm_internal.c (xenXMDomainDefineXML): Dead store to "olddomain" --
> and comment ;-) highlighted that the virGetDomain call represented a
> leak. It was also useless, seeing as how preceding call to
> virHashLookup(priv->nameConfigMap found def->name.
> ---
> src/xm_internal.c | 5 -----
> 1 files changed, 0 insertions(+), 5 deletions(-)
>
> diff --git a/src/xm_internal.c b/src/xm_internal.c
> index de3aca9..e7f6a55 100644
> --- a/src/xm_internal.c
> +++ b/src/xm_internal.c
> @@ -2532,7 +2532,6 @@ cleanup:
> */
> virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) {
> virDomainPtr ret;
> - virDomainPtr olddomain;
> char filename[PATH_MAX];
> const char * oldfilename;
> virDomainDefPtr def = NULL;
> @@ -2578,10 +2577,6 @@ virDomainPtr xenXMDomainDefineXML(virConnectPtr conn, const char *xml) {
> goto error;
> }
>
> - /* XXX wtf.com is this line for - it appears to be amemory leak */
> - if (!(olddomain = virGetDomain(conn, def->name, entry->def->uuid)))
> - goto error;
> -
> /* Remove the name -> filename mapping */
> if (virHashRemoveEntry(priv->nameConfigMap, def->name, NULL) < 0) {
> xenXMError(conn, VIR_ERR_INTERNAL_ERROR,
Hum .... not so fast :-)
so we are defining a domain, it's in the config map, but if we try to
retrieve it it doesn't show up ...
I wonder if it wasn't a check to preserve files in /etc/xen which
were not domain descriptions, so that someone would not override
a non-domain config file.
Historical, and nasty, I would instead call virUnrefDomain(olddomain);
just after (assuming the virGetDomain didn't find NULL).
But that would need testing, I guess :-\
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