Re: [Xen-devel] [PATCH 05/10] libxl: Fix leaks on context init failure

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2012-01-06 at 20:35 +0000, Ian Jackson wrote:
> Several of the error exits from libxl_ctx_alloc leaked the context
> struct itself and sometimes other resources too.
> 
> Fix this by using the standard "rc = ERROR_FOO; goto out" error
> handling style throughout.
> 
> Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Acked-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

> ---
>  tools/libxl/libxl.c |   20 ++++++++++++--------
>  1 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> index 2d3e8cd..8ecce26 100644
> --- a/tools/libxl/libxl.c
> +++ b/tools/libxl/libxl.c
> @@ -24,17 +24,17 @@
>  int libxl_ctx_alloc(libxl_ctx **pctx, int version,
>                      unsigned flags, xentoollog_logger * lg)
>  {
> -    libxl_ctx *ctx;
> +    libxl_ctx *ctx = NULL;
>      struct stat stat_buf;
>      const pthread_mutex_t mutex_value = 
> PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
> +    int rc;
>  
> -    if (version != LIBXL_VERSION)
> -        return ERROR_VERSION;
> +    if (version != LIBXL_VERSION) { rc = ERROR_VERSION; goto out; }

Aside: we really ought to have been cranking this number and probably
the SONAME too. 4.2 should definitely differ from 4.1 in at least one of
those, if not both. 

Ian.

>  
>      ctx = malloc(sizeof(*ctx));
>      if (!ctx) {
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Failed to allocate 
> context\n");
> -        return ERROR_NOMEM;
> +        rc = ERROR_NOMEM; goto out;
>      }
>  
>      memset(ctx, 0, sizeof(libxl_ctx));
> @@ -48,14 +48,14 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
>      if ( stat(XENSTORE_PID_FILE, &stat_buf) != 0 ) {
>          LIBXL__LOG_ERRNO(ctx, LIBXL__LOG_ERROR, "Is xenstore daemon 
> running?\n"
>                       "failed to stat %s", XENSTORE_PID_FILE);
> -        return ERROR_FAIL;
> +        rc = ERROR_FAIL; goto out;
>      }
>  
>      ctx->xch = xc_interface_open(lg,lg,0);
>      if (!ctx->xch) {
>          LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, errno,
>                          "cannot open libxc handle");
> -        return ERROR_FAIL;
> +        rc = ERROR_FAIL; goto out;
>      }
>  
>      ctx->xsh = xs_daemon_open();
> @@ -64,12 +64,16 @@ int libxl_ctx_alloc(libxl_ctx **pctx, int version,
>      if (!ctx->xsh) {
>          LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, errno,
>                          "cannot connect to xenstore");
> -        xc_interface_close(ctx->xch);
> -        return ERROR_FAIL;
> +        rc = ERROR_FAIL; goto out;
>      }
>  
>      *pctx = ctx;
>      return 0;
> +
> + out:
> +    libxl_ctx_free(ctx);
> +    *pctx = NULL;
> +    return rc;
>  }
>  
>  int libxl_ctx_free(libxl_ctx *ctx)



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel