[libvirt] leak? xenUnifiedOpen

[Daniel Veillard]

On Mon, Jul 21, 2008 at 10:27:45AM +0100, Daniel P. Berrange wrote:
> On Mon, Jul 21, 2008 at 04:52:24AM -0400, Daniel Veillard wrote:
> > On Sun, Jul 20, 2008 at 10:49:39PM +0200, Stefan de Konink wrote:
> > > ==5486== 184 bytes in 1 blocks are definitely lost in loss record 5 of 18
> > > ==5486==    at 0x4C20111: calloc (vg_replace_malloc.c:397)
> > > ==5486==    by 0x4E3ADFD: __virAlloc (memory.c:100)
> > > ==5486==    by 0x4E4AF21: xenUnifiedOpen (xen_unified.c:264)
> > > ==5486==    by 0x4E3A005: do_open (libvirt.c:777)
> > > ==5486==    by 0x4009D7: main (in 
> > > /home/skinkie/development/htdocs/cherokee/experiment)
> > > 
> > > 
> > > Maybe valgrind is over active, because I see the free code at the close 
> > > connection, and my own code is closing it...
> > 
> >   I can see a possibility, for example if xenUnifiedOpen is called
> > multipl time on the same connection but the first time it exits on
> > line 332 of xenUnifiedOpen(). That error exit leaves the routine
> > without deallocating the priv block and conn->privateData is not
> > set back to NULL.
> 
> Yes, we definitely need to free the 'priv' data  on line 332.

  I suggest the following

Daniel

-- 
Red Hat Virtualization group http://redhat.com/virtualization/
Daniel Veillard      | virtualization library  http://libvirt.org/
veillard at redhat.com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine  http://rpmfind.net/
-------------- next part --------------
Index: src/xen_unified.c
===================================================================
RCS file: /data/cvs/libxen/src/xen_unified.c,v
retrieving revision 1.47
diff -u -p -p -r1.47 xen_unified.c
--- src/xen_unified.c	6 Jun 2008 11:09:57 -0000	1.47
+++ src/xen_unified.c	21 Jul 2008 09:35:44 -0000
@@ -239,7 +239,7 @@ xenUnifiedProbe (void)
 static int
 xenUnifiedOpen (virConnectPtr conn, xmlURIPtr uri, virConnectAuthPtr auth, int flags)
 {
-    int i;
+    int i, ret = VIR_DRV_OPEN_DECLINED;
     xenUnifiedPrivatePtr priv;
 
     /* Refuse any scheme which isn't "xen://" or "http://". */
@@ -329,7 +329,8 @@ xenUnifiedOpen (virConnectPtr conn, xmlU
             }
 #else
             DEBUG0("Handing off for remote driver");
-            return VIR_DRV_OPEN_DECLINED; /* Let remote_driver try instead */
+            ret = VIR_DRV_OPEN_DECLINED; /* Let remote_driver try instead */
+	    goto clean;
 #endif
         }
     }
@@ -337,11 +338,13 @@ xenUnifiedOpen (virConnectPtr conn, xmlU
     return VIR_DRV_OPEN_SUCCESS;
 
  fail:
+    ret = VIR_DRV_OPEN_ERROR;
+clean:
     DEBUG0("Failed to activate a mandatory sub-driver");
     for (i = 0 ; i < XEN_UNIFIED_NR_DRIVERS ; i++)
         if (priv->opened[i]) drivers[i]->close(conn);
     VIR_FREE(priv);
-    return VIR_DRV_OPEN_ERROR;
+    return ret
 }
 
 #define GET_PRIVATE(conn) \