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

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



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<anthony.perard@xxxxxxxxxx> 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
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

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