[libvirt] [PATCH 3/5] storage_conf: Various fixes or improvements on pool def parsing

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130520/d57f62b0/attachment-0001.sig>