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.per...@citrix.com>
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
