Re: [Xen-devel] [PATCH 08/13 v5] libxl: don't leak ptr in libxl_list_vm error case

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Tue, 2013-12-03 at 14:29 +1300, Matthew Daley wrote:
> While at it, tidy up the function; there's no point in allocating more
> than the amount of domains actually returned by xc_domain_getinfolist
> (barring the caveat described in the newly-added comment)
> 
> Coverity-ID: 1055888
> Signed-off-by: Matthew Daley <mattd@xxxxxxxxxxx>
> ---
> v5: Use libxl__calloc instead of calloc
> 
>  tools/libxl/libxl.c |   27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 67a8e0e..3b73d99 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -671,20 +671,24 @@ out:
>   * be an aggregate of multiple domains. */
>  libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int *nb_vm_out)
>  {
> -    libxl_vminfo *ptr;
> +    GC_INIT(ctx);
> +    libxl_vminfo *ptr = NULL;
>      int idx, i, ret;
>      xc_domaininfo_t info[1024];
> -    int size = 1024;
>  
> -    ptr = calloc(size, sizeof(libxl_vminfo));
> -    if (!ptr)
> -        return NULL;
> -
> -    ret = xc_domain_getinfolist(ctx->xch, 1, 1024, info);
> -    if (ret<0) {
> -        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "geting domain info list");
> -        return NULL;
> +    ret = xc_domain_getinfolist(ctx->xch, 1, ARRAY_SIZE(info), info);
> +    if (ret < 0) {
> +        LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "getting domain info list");
> +        goto out;
>      }
> +
> +    /*
> +     * Always make sure to allocate at least one element; if we don't and we
> +     * request zero, libxl__calloc (might) think its internal call to calloc
> +     * has failed (if it returns null), if so it would kill our process.

Is size==0 something we could/should handle in our libxl__*alloc
wrappers?

Or maybe this is something we should handle here e.g. by returning NULL,
except perhaps our API doesn't allow for that?

> +     */
> +    ptr = libxl__calloc(NOGC, ret ? ret : 1, sizeof(libxl_vminfo));
> +
>      for (idx = i = 0; i < ret; i++) {
>          if (libxl_is_stubdom(ctx, info[i].domain, NULL))
>              continue;
> @@ -694,6 +698,9 @@ libxl_vminfo * libxl_list_vm(libxl_ctx *ctx, int 
> *nb_vm_out)
>          idx++;
>      }
>      *nb_vm_out = idx;
> +
> +out:
> +    GC_FREE;
>      return ptr;
>  }
>  



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