Re: [Xen-devel] [PATCH 2 of 7 v2] blktap3/tapback: Introduces functionality required to access XenStore

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-01-04 at 12:14 +0000, Thanos Makatos wrote:
> This patch introduces convenience functions that read/write values from/to
> XenStore.
> 
> diff -r 0777319f0a6e -r efd3f343f7c7 tools/blktap3/tapback/xenstore.c
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/tools/blktap3/tapback/xenstore.c        Fri Jan 04 11:54:51 2013 +0000
> @@ -0,0 +1,188 @@
> +/*
> + * Copyright (C) 2012      Citrix Ltd.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version 2
> + * of the License, or (at your option) any later version.
> + * 
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + * 
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
> + * USA.
> + */
> +
> +#include <stdarg.h>
> +#include <stdio.h>
> +#include <xenstore.h>
> +#include <assert.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +#include "blktap3.h"
> +#include "tapback.h"
> +#include "xenstore.h"
> +
> +/**
> + * Prints the supplied arguments to a buffer and returns it. The buffer must
> + * be deallocated by the user.

I'm not sure how much value this function has for callers over and above
just using (v)asprintf directly, likewise for mprintf.

> + */
> +static char *
> +vmprintf(const char * const fmt, va_list ap)
> +{
> +    char *s = NULL;

No need to init s.

> +
> +    if (vasprintf(&s, fmt, ap) < 0)
> +        s = NULL;
> +
> +    return s;
> +}
> +
> +__printf(1, 2)
> +char *
> +mprintf(const char * const fmt, ...)
> +{
> +    va_list ap;
> +    char *s = NULL;

No need to init s.

> +
> +    va_start(ap, fmt);
> +    s = vmprintf(fmt, ap);
> +    va_end(ap);
> +
> +    return s;
> +}
> +
> +char *
> +tapback_xs_vread(struct xs_handle * const xs, xs_transaction_t xst,
> +        const char * const fmt, va_list ap)

This might be an interesting/useful addition to libxenstore?

> +{
> +    char *path = NULL, *data = NULL;

No need to init path.

> +    unsigned int len = 0;
> +
> +    assert(xs);
> +
> +    path = vmprintf(fmt, ap);
> +    data = xs_read(xs, xst, path, &len);

This uses path without checking if it is NULL.

I think you can pass len == NULL if you don't care about the length of
the result.

However xenstore values generally can contain nulls and are not
necessarily NULL terminated (see docs/misc/xenstore.txt), but perhaps
the block protocol tightens this restriction such that you can rely on
NULL termination (at least in practice)?

> +    free(path);
> +
> +    return data;
> +}
> +
> +__printf(3, 4)
> +char *
> +tapback_xs_read(struct xs_handle * const xs, xs_transaction_t xst,
> +        const char * const fmt, ...)
> +{
> +    va_list ap;
> +    char *s = NULL;

No need to init s.

> +
> +    assert(xs);
> +
> +    va_start(ap, fmt);
> +    s = tapback_xs_vread(xs, xst, fmt, ap);
> +    va_end(ap);
> +
> +    return s;
> +}
> +

> +__scanf(3, 4)
> +int
> +tapback_device_scanf_otherend(vbd_t * const device,
> +        const char * const path, const char * const fmt, ...)
> +{
> +    va_list ap;
> +    int n = 0;
> +    char *s = NULL;
> +
> +    assert(device);
> +    assert(path);
> +
> +    if (!(s = tapback_device_read_otherend(device, path)))
> +        return -1;

Since s here is read from the frontend you probably can't trust it to be
NULL terminated.

> +    va_start(ap, fmt);
> +    n = vsscanf(s, fmt, ap);
> +    free(s);
> +    va_end(ap);
> +
> +    return n;
> +}
> +

Ian.


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