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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Andrew Cooper writes ("[Xen-devel] [Patch v3 3/4] tools/libxl: Fix 
libxl__device_nic_from_xs_be()"):
> 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.

Thanks,
Ian.

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