Re: [Xen-devel] [Patch v3 3/4] tools/libxl: Fix libxl__device_nic_from_xs_be()

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-11-26 at 15:15 +0000, Andrew Cooper wrote:
> On 26/11/13 15:08, Ian Jackson wrote:
> > Andrew Cooper writes ("Re: [Xen-devel] [Patch v3 3/4] tools/libxl: Fix 
> > libxl__device_nic_from_xs_be()"):
> >> Commit message should include the Coverity ID 1055886, and perhaps a
> >> reference to the fact that it is a memory leak.
> > ...
> >>> +    nic->ip = READ_BACKEND("ip");
> >>> +    nic->bridge = READ_BACKEND("bridge");
> >>> +    nic->script = READ_BACKEND("script");
> >> This is not correct.  libxl_device_nic_dispose() is in charge of freeing
> >> these pointers, but now they are part of the gc.
> > Oops.
> >
> > From 5bdaef8453bc8fd06da956df90fe33a12190342c Mon Sep 17 00:00:00 2001
> > From: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> > Date: Tue, 26 Nov 2013 12:08:09 +0000
> > Subject: [PATCH] libxl: Fix error handling in libxl__device_nic_from_xs_be
> >
> > Previously, this function had a memory leak.  Fix this, and the rest
> > of the error handling.

I had to go back to the original to figure out what the memory leak was,
and so I rewrote this as:

    Previously, this function would leak the temporary return from xs_read for
    handle and mac address.  Fix both of these and the rest of the error 
handling.
    
> >
> > This requires changing its return type and fixing the callers.
> >
> > Introduce here a READ_BACKEND macro to make the code less repetitive.
> >
> > Coverity ID: 1055886
> >
> > Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> 
> This looks like it might plausibly work.
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Acked + committed with the above.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel