Re: [Xen-devel] [PATCH 0 of 3] libxl: memory leaks

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Tue, 3 Aug 2010, Ian Campbell wrote:
> On Tue, 2010-08-03 at 14:37 +0100, Vincent Hanquez wrote:
> > On 03/08/10 13:16, Gianni Tedesco (3P) wrote:
> > > I actually prefer explicit free's on the returned objects. That gives
> > > callers a lot more control. Have you seen Ians patch auto-generating
> > > that code? I think this approach combined with automatic-freeing of
> > > scratch data used in libxl calls is the best of both worlds.
> > >    
> > How much control do you actually need ?
> > 
> > In python you'ld have the approach:
> 
> > my_function_xl_binded()
> > {
> >   fill_structure(&structure);
> >   CTX_INIT;
> >   do_xl_call(&structure);
> >   pyval= convert_to_python_values(&structure);
> >   CTX_FREE;
> 
>     free_structure(&structure); // free stuff dynamically allocated by 
> fill_structure 
> 
> >   return pyval;
> > }
> 
> This gets more complicated if do_xl_call may have potentially
> reallocated or otherwise changed some members of &structure (I think we
> had a similar conversation to this one in the context of
> libxl_run_bootloader)
> 
> How does libxl know how it should free the existing value before
> replacing it (or should it just add it to the context instead?). How
> does the caller know if/when this has happened and how does it then go
> about freeing it or not as necessary on cleanup?
> 
> Mandating that all data passed over the libxl call boundary must be
> explicitly freed avoids all of this, is more in keeping with how people
> expect a library to behave and isn't really any more lines of code than
> the above (at least given the presence of free_foo() style helpers,
> which it turns out are needed anyway).
> 
> The alternative is to go completely the other way and mandate all data
> passed of the boundary must be registered with the context, which would
> entail passing a ctx to fill_structure (and maybe convert_to_... as
> well), sprinkling libxl_ptr_add around the place, exporting libxl_strdup
> etc etc. I'm not sure where the win is here.
> 
> Mixing and matching these two schemes (plus mixing in static data),
> which is what xl and the ocaml bindings both do today, just isn't
> workable if we expect people to consistently have a decent chance of
> writing correct programs using the library.
> 

Libxl should be as easy to use as possible, even by people not following
xen-devel, for this reason I really like the explicit frees, simply
because it is how everybody else does it, no risks of being
misunderstood.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel