Skip to main content

Request for Comment – xl: start implementing "the policy" wrt memory

By August 9, 2010March 4th, 2019Uncategorized
Full RFC Patch -
It is libxl policy to return objects to callers which the caller must
free. It therefore a matter of correctness for libxl to only free it's
internal scratch allocations and never to free things which may be
returned so as to avoid double-free error conditions.
This patch (probably won't apply and is dirty as hell for now) - begins
to implement this. The approach taken is to use gcc's visibility
attribute to hide libxl internal functions from callers. This includes
the libxl garbage collection routines. These routines now take a new
structure libxl_gc which contains the garbage collection variables and a
back-pointer to the associated libxl context. Other changes include
returning void from libxl_*free* and abort()'ing when it goes wrong.
Working on this patch exposed numerous violations of the policy:
 - libxl_domid_to_name()
 - libxl_get_version_info()
 - libxl_list_vcpu()
 - libxl_list_nics()
 - libxl_device_net2_list()
 - perhaps others
domid_to_name has numerous in and out of library callers so for that I
introduced a libxl-internal _libxl_domid_to_name() which participates in
libxl garbage collection - a little too subtle for my taste but perhaps
something to be lived with.
The others would benefit from destructor functions such as Ian
Campbell's patch introduced.
A side effect of the gcc visibility stuff is getting rid of about a
thousand GOT/PLT entries which shaves 1-2KB off the binary size and
ought to speed up dynamic linking.
What needs to be done from here is a thorough audit of the policy and a
lot of time alone in a dark room doing bad things to xl with valgrind :)
I am thinking eventually of 4 logical changesets:
 1. Fix flagrant policy violations
 2. Get rid of leakiness with the new gc stuff (bulk of patch)
 3. Sprinkle the _hidden macros around
 4. Not decided how best to handle xs vs. NULL paths yet...
    probably xs_$(OP)f() w/ printf semantics built-in?