Re: [Xen-devel] [PATCH 01 of 23] libxl: Document _init/_dispose/_setdefault functions

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2012-02-20 at 19:08 +0000, Ian Jackson wrote:
> Ian Campbell writes ("[Xen-devel] [PATCH 01 of 23] libxl: Document 
> _init/_dispose/_setdefault functions"):
> > + * void libxl_<type>_init_<subfield>(<type> *p, subfield):
> > + *
> > + *    Initialise those parts of "p" which are not initialised by the
> > + *    main init function due to the unknown value of "subfield". Sets
> > + *    p->subfield as well as initialising any fields to their default
> > + *    values.
> > + *
> > + *    p->subfield must not have been previously initialised.
> > + *
> > + *    This method is provided for any aggregate type which is used as
> > + *    an input parameter.
> 
> This final sentence needs a qualification I think.

I struggled a bit to express what I meant.

What I was trying to say is that they are produced only for types
declared in the idl with dir=IN|BOTH. There are some output only types
which a user cannot reasonably create.

Maybe I should just declare that the distinction is irrelevant and
always generate an _init? (I think I introduced it because of some issue
I was having with the code generation, I'll try again and see if I trip
over it again)

> > + * void libxl_<type>_dispose(instance *p):
> > + *
> > + *    Frees any dynamically allocated memory used by the members of
> > + *    "p" but not the storage used by "p" itself (this allows for the
> > + *    allocation of arrays of types and for the composition of types).
> > + *
> > + *    In general this method is only provided for types where memory
> > + *    can be dynamically allocated as part of that type.
> 
> Is whether dynamic allocation may happen a very stable part of the
> libxl API ?  If we add a dynamically allocated member to one of these
> structs, don't we inevitably introduce a memory leak into all
> callers ?
> 
> I think it would be better to provide a dispose function corresponding
> to every init function.

I think the _dispose functions should be a strict superset since you
need to be able to dispose of output-only functions too. I _think_ that
means we should have a _dispose for every type.

I expect we'll uncover a mass of latent bugs whenever we add dynamic
allocation to a type which previously did not do so. But at least the
bug fix will be compatible with older versions of the library too, so I
will go ahead and add a dispose everywhere.

Ian.


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