[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [Patch v3 3/4] tools/libxl: Fix libxl__device_nic_from_xs_be()

Andrew Cooper writes ("[Xen-devel] [Patch v3 3/4] tools/libxl: Fix 
> diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
> -    tmp = xs_read(ctx->xsh, XBT_NULL,
> -                  libxl__sprintf(gc, "%s/handle", be_path), &len);
> -    if ( tmp )
> +    rc = libxl__xs_read_checked(gc, XBT_NULL,
> +                                libxl__sprintf(gc, "%s/handle", be_path),
> +                                &tmp);
> +
> +    if ((rc == 0) && strlen(tmp))

Nacked-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
(for the benefit of Ian C.)

This is not correct.  See the doc comment for libxl__xs_read_checked:

    /* On success, *result_out came from the gc.
     * On error, *result_out is undefined.
     * ENOENT counts as success but sets *result_out=0
    int libxl__xs_read_checked(libxl__gc *gc, xs_transaction_t t,
                               const char *path, const char **result_out);

So the correct pattern is:

    rc = libxl__xs_read_checked(gc, XBT_NULL, blah blah blah, &tmp);
    if (rc) goto out;

    if (tmp) {
        use tmp;
    } else {
        the path doesn't exist, do the other thing;

I don't think there should be any need to check for empty strings
written to xenstore here ?  The old code doesn't.  Please someone tell
me there isn't.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.