Xen devel - Jun 2012 - [PATCH] libxl: document the memory ownership of some functions

Specifying they allocate dynamic memory that needs to be explicitly freed.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -586,8 +586,16 @@ int libxl_primary_console_get_tty(libxl_
 int libxl_domain_info(libxl_ctx*, libxl_dominfo *info_r,
                       uint32_t domid);
 libxl_dominfo * libxl_list_domain(libxl_ctx*, int *nb_domain);
+  /* On success, a list of nb_domain libxl_dominfo elements is
+   * returned. That comes from malloc, thus it is up to the caller
+   * to invoke libxl_dominfo_list_free() on it.
+   */
 void libxl_dominfo_list_free(libxl_dominfo *list, int nr);
 libxl_cpupoolinfo * libxl_list_cpupool(libxl_ctx*, int *nb_pool);
+  /* On success, a list of nb_pool libxl_cpupoolinfo elements is
+   * returned. That comes from malloc, thus it is up to the caller
+   * to invoke libxl_cpupoolinfo_list_free() on it.
+   */
 void libxl_cpupoolinfo_list_free(libxl_cpupoolinfo *list, int nr);
 libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int *nb_vm);
 void libxl_vminfo_list_free(libxl_vminfo *list, int nr);
@@ -768,9 +776,18 @@ int libxl_userdata_retrieve(libxl_ctx *c
 int libxl_get_physinfo(libxl_ctx *ctx, libxl_physinfo *physinfo);
 #define LIBXL_CPUTOPOLOGY_INVALID_ENTRY (~(uint32_t)0)
 libxl_cputopology *libxl_get_cpu_topology(libxl_ctx *ctx, int *nr);
+  /* On success, the actual machine topology is returned as a
+   * list of nr libxl_cputopology elements. That comes from malloc,
+   * thus it is up to the caller to invoke libxl_cputopology_list_free()
+   * on it.
+   */
 void libxl_cputopology_list_free(libxl_cputopology *, int nr);
 libxl_vcpuinfo *libxl_list_vcpu(libxl_ctx *ctx, uint32_t domid,
                                        int *nb_vcpu, int *nrcpus);
+  /* On success, a list of nrcpus libxl_vcpuinfo elements is
+   * returned. That comes from malloc, thus it is up to the
+   * caller to invoke libxl_vcpuinfo_list_free() on it.
+   */
 void libxl_vcpuinfo_list_free(libxl_vcpuinfo *, int nr);
 int libxl_set_vcpuaffinity(libxl_ctx *ctx, uint32_t domid, uint32_t vcpuid,
                            libxl_cpumap *cpumap);

Dario Faggioli writes ("[PATCH] libxl: document the memory ownership of
some functions"):
> Specifying they allocate dynamic memory that needs to be explicitly freed.
Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

But, just a suggestion:
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> 
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
Perhaps it would be worth writing this comment once near the top, eg:

 /* These functions each return (on success) an array of elements,
  * and the length via the int* out parameter.  These arrays and
  * their contents come from malloc, and must be freed with the
  * corresponding libxl_THING_list_free function.
  */

And perhaps change  int *nb_domain  etc. to  int *nb_domain_out.

Ian.

On Fri, 2012-06-15 at 15:11 +0100, Ian Jackson wrote:
> Perhaps it would be worth writing this comment once near the top, eg:
> 
>  /* These functions each return (on success) an array of elements,
>   * and the length via the int* out parameter.  These arrays and
>   * their contents come from malloc, and must be freed with the
>   * corresponding libxl_THING_list_free function.
>   */
> 
> And perhaps change  int *nb_domain  etc. to  int *nb_domain_out.
> 
I like this, and I''m fine with going for this and respending. Just to
be
sure, I''d need to gather all those functions together, moving them from
their current positions in the header... Would that be fine?

Thanks and Regards,
Dario

-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://retis.sssup.it/people/faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Dario Faggioli writes ("Re: [PATCH] libxl: document the memory ownership of
some functions"):
> On Fri, 2012-06-15 at 15:11 +0100, Ian Jackson wrote:
> > Perhaps it would be worth writing this comment once near the top, eg:
> > 
> >  /* These functions each return (on success) an array of elements,
> >   * and the length via the int* out parameter.  These arrays and
> >   * their contents come from malloc, and must be freed with the
> >   * corresponding libxl_THING_list_free function.
> >   */
> > 
> > And perhaps change  int *nb_domain  etc. to  int *nb_domain_out.
> 
> I like this, and I''m fine with going for this and respending. Just
to be
> sure, I''d need to gather all those functions together, moving them
from
> their current positions in the header... Would that be fine?
Moving the functions is fine if it seems like the new order is
reasonable.  I haven''t checked...

Ian.