Juergen Gross
2010-Dec-09 13:51 UTC
[Xen-devel] [PATCH] add missing libxl__free_all() calls
In various libxl functions libxl__free_all() was missing before return Signed-off-by: juergen.gross@ts.fujitsu.com 3 files changed, 39 insertions(+), 12 deletions(-) tools/libxl/libxl.c | 34 ++++++++++++++++++++++++++-------- tools/libxl/libxl_dom.c | 13 ++++++++++--- tools/libxl/libxl_pci.c | 4 +++- _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Gianni Tedesco
2010-Dec-09 14:12 UTC
Re: [Xen-devel] [PATCH] add missing libxl__free_all() calls
On Thu, 2010-12-09 at 13:51 +0000, Juergen Gross wrote:> @@ -3760,8 +3772,10 @@ int libxl_destroy_cpupool(libxl_ctx *ctx > libxl_cpumap cpumap; > > info = xc_cpupool_getinfo(ctx->xch, poolid); > - if (info == NULL) > - return ERROR_NOMEM; > + if (info == NULL) { > + libxl__free_all(&gc); > + return ERROR_NOMEM; > + }This one is un-necessary but harmless. The INIT_GC macro just zeros the structure and allocates nothing. It might not be wise to rely on programmer knowing that about the implementation in the long run...> rc = ERROR_INVAL; > if ((info->cpupool_id != poolid) || (info->n_dom)) > @@ -3805,6 +3819,7 @@ out1: > libxl_cpumap_destroy(&cpumap); > out: > xc_cpupool_infofree(ctx->xch, info); > + libxl__free_all(&gc); > > return rc; > } > @@ -3963,6 +3978,7 @@ int libxl_cpupool_movedomain(libxl_ctx * > > dom_path = libxl__xs_get_dompath(&gc, domid); > if (!dom_path) { > + libxl__free_all(&gc); > return ERROR_FAIL; > }Same again. All the others are correct, most of them obviously so just from the diff context, making them good catches. Nice work. Gianni _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Stefano Stabellini
2010-Dec-13 18:14 UTC
Re: [Xen-devel] [PATCH] add missing libxl__free_all() calls
On Thu, 9 Dec 2010, Gianni Tedesco wrote:> On Thu, 2010-12-09 at 13:51 +0000, Juergen Gross wrote: > > @@ -3760,8 +3772,10 @@ int libxl_destroy_cpupool(libxl_ctx *ctx > > libxl_cpumap cpumap; > > > > info = xc_cpupool_getinfo(ctx->xch, poolid); > > - if (info == NULL) > > - return ERROR_NOMEM; > > + if (info == NULL) { > > + libxl__free_all(&gc); > > + return ERROR_NOMEM; > > + } > > This one is un-necessary but harmless. The INIT_GC macro just zeros the > structure and allocates nothing. It might not be wise to rely on > programmer knowing that about the implementation in the long run... >I actually think that is a good idea to always call libxl__free_all before exiting. libxl__free_all should guarantee to do the right thing no matter if something has been allocated or not, so I am going to apply the patch as is. Thanks! Stefano _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel