Re: [libvirt] [PATCH 3/5] storage_conf: Various fixes or improvements on pool def parsing
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
Attachments:
- signature.asc (application/pgp-signature — 621 bytes)