[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]