Re: [libvirt] [PATCH 12/12] storage_conf: Use uid_t/gid_t instead of int to cast the value

On 05/22/2013 06:05 AM, Osier Yang wrote: > And error out if the casted value is not same with the original > one, which prevents the bug on platform(s) where uid_t/gid_t > has different size with long. > --- > src/conf/storage_conf.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > } else { > - if (virXPathLong("number(./owner)", ctxt, &v) < 0) { > + if (virXPathLong("number(./owner)", ctxt, &val) < 0 || > + (uid_t)val != val) { Once you have made this check... > virReportError(VIR_ERR_XML_ERROR, "%s", > _("malformed owner element")); > goto error; > } > - perms->uid = (int)v; > + > + perms->uid = (uid_t)val; ...the cast here is redundant. You could write 'perms->uid = val'. > } > > if (virXPathNode("./group", ctxt) == NULL) { > perms->gid = (gid_t) -1; > } else { > - if (virXPathLong("number(./group)", ctxt, &v) < 0) { > + if (virXPathLong("number(./group)", ctxt, &val) < 0 || > + (gid_t)val != val) { > virReportError(VIR_ERR_XML_ERROR, "%s", > _("malformed group element")); > goto error; > } > - perms->gid = (int)v; > + perms->gid = (gid_t)val; Likewise. ACK with that simplification, and with your followup that explicitly allows -1.