Re: [Qemu-devel] [Xen-devel] [PATCH 01/19] xen: Create a new file xen_pvdev.c

[Emil Condrea]

Sure, I will continue to send revisions until it is approved upstream.
On Jul 22, 2016 5:24 PM, "Quan Xu" <address@hidden> wrote:

> Anthony, thanks for your explaination.
> IMO, patch 1  and patch 2 need your detailed review.. IMO the reset
> patches are good in general..
> Emil, if patch 1 / patch 2 are reviewed from anthony, could you send out
> v10? :) i know it's not an easy task, thanks in advence!!
>
> Quan
>
>
> On Mon, 18 Jul 2016 15:50:27 +0100, anthony.perard<
> address@hidden> wrote:
> > On Sun, Jul 17, 2016 at 03:41:26PM +0800, Quan Xu wrote:
> > -int xenstore_write_int(const char *base, const char *node, int ival)
> > -{
> > -    char val[12];
> > -
> > [Quan:]: why 12 ? what about XEN_BUFSIZE?
>
> That is the number of digit in INT_MAX (10) + 1 for the sign + 1 for '\0'.
>
> > -    snprintf(val, sizeof(val), "%d", ival);
> > -    return xenstore_write_str(base, node, val);
> > -}
> > -
>
> > -int xenstore_write_int64(const char *base, const char *node, int64_t ival)
> > -{
> > -    char val[21];
> > -
> > [Quan:]: why 21 ? what about XEN_BUFSIZE?
>
> Same with INT64_MAX (19 digits).
>
> >
> > -    snprintf(val, sizeof(val), "%"PRId64, ival);
> > -    return xenstore_write_str(base, node, val);
> > -}
> > -
> > -int xenstore_read_int(const char *base, const char *node, int *ival)
> > -{
> > -    char *val;
> > -    int rc = -1;
> > -
> > -    val = xenstore_read_str(base, node);
> > [Quan:]:  IMO, it is better to initialize val when declares.
>
> I think I prefer it this way.
>
> > -    if (val && 1 == sscanf(val, "%d", ival)) {
> > -        rc = 0;
> > -    }
> > -    g_free(val);
> > -    return rc;
> > -}
>
> --
> Anthony PERARD
>