On 05/16/2013 06:40 AM, Osier Yang wrote:
virStorageDefParsePerms: * Use uid_t/gid_t to do casting
@@ -780,22 +785,22 @@ virStorageDefParsePerms(xmlXPathContextPtr ctxt, perms->uid = (uid_t) -1; } else { if (virXPathLong("number(./owner)", ctxt, &v) < 0) { - virReportError(VIR_ERR_XML_ERROR, - "%s", _("malformed owner element")); + virReportError(VIR_ERR_XML_ERROR, "%s", + _("malformed owner element")); goto error; } - perms->uid = (int)v; + perms->uid = (uid_t)v;
As Daniel said, please separate uid_t/gid_t bug fixes into their own patch, as it is one of my trigger words for a patch needing extra careful review, and yet I had to hunt for where you were doing it. Still buggy, but now in a different way. Pre-patch, if you have a 64-bit long and 64-bit uid_t (is there such a platform? I don't know of one), you were silently throwing away the most significant bits. But you are not guaranteed that uid_t and long are the same size. On another platform where uid_t is 32-bit but long is 64-bit (and Linux on x86_64 is such a platform), then when I pass in 0x100000000, you should reject it as invalid, rather than silently truncating to 0. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org