Sure, I will continue to send revisions until it is approved upstream. On Jul 22, 2016 5:24 PM, "Quan Xu" <quan....@aliyun.com> 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< > 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 >