[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] libxl: Use libxl_strdup instead of strdup on libxl_version_info
On Thu, 2016-01-14 at 22:03 -0500, Konrad Rzeszutek Wilk wrote: > On January 14, 2016 9:33:49 PM EST, Konrad Rzeszutek Wilk <konrad.wilk@or > acle.com> wrote: > > As the libxl_strdup allows us to unwind and free all > > of the allocations, while strdup requires the callers > > to remember to free (which they didn't seem too). > > > > > Grrrrr.. Ignore it pls. I cherry picked it and had a preceding patch that > added the GC_INIT which this patch missed. > > (And also an GC_FREE). Allocating these returned values via a gc would be wrong per the comment about memory management at the top of libxl.h, where these fall into category (b) and require the caller to free. They should normally do so by calling libxl_version_info_dispose(), not manually (which would be quite tedious and error prone). Returning GC'd values to the application would be wrong since they would be freed by the GC_FREE before return upon exit from libxl. However you've used NOGC, which compared to raw strdup etc only has the extra behaviour of abort-on-alloc-fail and not any "unwind and free all" behaviour which the commit message mentions. So this would be a good change, in that it improves error handling, rather than for any of the reasons mentioned in the commit message. WRT freeing the results, normally the caller would need to do so by calling the appropriate _dispose() function. HoweverÂlibxl_get_version_info() is special and returns a cached result from the ctx which cannot and should not be freed (as evidenced by it returning a const struct). This data is freed in libxl_ctx_free() by callingÂlibxl_version_info_dispose(). This is why none of the callers remember to free -- they shouldn't be doing so. Ian. _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |