[libvirt] [PATCH] xm_internal.c: remove dead store and associated leak

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@redhat.com> 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, -- 1.6.4.2.419.gab238

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@redhat.com> 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@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/
participants (2)
-
Daniel Veillard
-
Jim Meyering