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

[Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>]

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.

Ian.



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