Re: [Xen-devel] [PATCH 04/13 v2] libxl: don't leak p in libxl__wait_for_backend

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/12/2013 00:27, Matthew Daley wrote:
> Use libxl__xs_read_checked instead of xs_read. While at it, tidy up the
> function as well.
>
> Coverity-ID: 1055891
> Signed-off-by: Matthew Daley <mattd@xxxxxxxxxxx>
> ---
> v2: Use libxl__xs_read_checked instead of xs_read. Tidy up the function.
>
>  tools/libxl/libxl_device.c   |   41 +++++++++++++++++------------------------
>  tools/libxl/libxl_internal.h |    3 ++-
>  2 files changed, 19 insertions(+), 25 deletions(-)
>
> diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c
> index d995c83..ba7d100 100644
> --- a/tools/libxl/libxl_device.c
> +++ b/tools/libxl/libxl_device.c
> @@ -1199,37 +1199,30 @@ int libxl__wait_for_device_model_deprecated(libxl__gc 
> *gc,
>                                       check_callback, 
> check_callback_userdata);
>  }
>  
> -int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char *state)
> +int libxl__wait_for_backend(libxl__gc *gc, const char *be_path,
> +                            const char *state)
>  {
> -    libxl_ctx *ctx = libxl__gc_owner(gc);
>      int watchdog = 100;
> -    unsigned int len;
> -    char *p;
> -    char *path = GCSPRINTF("%s/state", be_path);
> -    int rc = -1;
> +    const char *p, *path = GCSPRINTF("%s/state", be_path);
> +    int rc;
> +
> +    while (watchdog-- > 0) {
> +        rc = libxl__xs_read_checked(gc, XBT_NULL, path, &p);

libxl__xs_read_checked() can return 0, with p set to NULL in the case of
an ENOENT from xenstore.

I think you need a NULL check before strcmp.

~Andrew

> +        if (rc) return rc;
>  
> -    while (watchdog > 0) {
> -        p = xs_read(ctx->xsh, XBT_NULL, path, &len);
>          if (p == NULL) {
> -            if (errno == ENOENT) {
> -                LOG(ERROR, "Backend %s does not exist", be_path);
> -            } else {
> -                LOGE(ERROR, "Failed to access backend %s", be_path);
> -            }
> -            goto out;
> -        } else {
> -            if (!strcmp(p, state)) {
> -                rc = 0;
> -                goto out;
> -            } else {
> -                usleep(100000);
> -                watchdog--;
> -            }
> +            LOG(ERROR, "Backend %s does not exist", be_path);
> +            return ERROR_FAIL;
>          }
> +
> +        if (!strcmp(p, state))
> +            return 0;
> +
> +        usleep(100000);
>      }
> +
>      LOG(ERROR, "Backend %s not ready", be_path);
> -out:
> -    return rc;
> +    return ERROR_FAIL;
>  }
>  
>  /*
> diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
> index a2d8247..1bd23ff 100644
> --- a/tools/libxl/libxl_internal.h
> +++ b/tools/libxl/libxl_internal.h
> @@ -944,7 +944,8 @@ _hidden char *libxl__device_frontend_path(libxl__gc *gc, 
> libxl__device *device);
>  _hidden int libxl__parse_backend_path(libxl__gc *gc, const char *path,
>                                        libxl__device *dev);
>  _hidden int libxl__device_destroy(libxl__gc *gc, libxl__device *dev);
> -_hidden int libxl__wait_for_backend(libxl__gc *gc, char *be_path, char 
> *state);
> +_hidden int libxl__wait_for_backend(libxl__gc *gc, const char *be_path,
> +                                    const char *state);
>  _hidden int libxl__nic_type(libxl__gc *gc, libxl__device *dev,
>                              libxl_nic_type *nictype);
>  


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