On 24/05/13 22:11, Eric Blake wrote:
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.
Thanks for the reviewing, I pushed the left patches (except 10/12) with the small nits pointed by you guys fixed. Osier