]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
maint: avoid potential promotion issues with [ug]id_t
authorEric Blake <eblake@redhat.com>
Mon, 7 Jan 2013 22:50:41 +0000 (15:50 -0700)
committerEric Blake <eblake@redhat.com>
Tue, 8 Jan 2013 15:45:39 +0000 (08:45 -0700)
POSIX does not guarantee whether uid_t and gid_t are signed or
unsigned, nor does it guarantee whether they are smaller, same
size, or larger than int (or even the same size as one another).
Therefore, it is possible to have platforms where '(uid_t)-1==-1'
is false or where 'uid = gid = -1' sets uid to the wrong value,
thanks to integer promotion rules.  The only portable way to use
the placeholder value of these two types is to always use a cast.
Thankfully, the issue is mostly theoretical - sanlock only
compiles on Linux for now, and on Linux, these types do not
suffer from strange promotion problems.

* src/locking/lock_driver_sanlock.c
(virLockManagerSanlockSetupLockspace, virLockManagerSanlockInit)
(virLockManagerSanlockCreateLease): Cast -1 to proper type before
comparing with uid_t or gid_t.

src/locking/lock_driver_sanlock.c

index c955003072e78ea37658dc2041bd393fdfb80549..d06fa661f0a5618240ee029391abec850f37a1fd 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * lock_driver_sanlock.c: A lock driver for Sanlock
  *
- * Copyright (C) 2010-2012 Red Hat, Inc.
+ * Copyright (C) 2010-2013 Red Hat, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -236,7 +236,7 @@ static int virLockManagerSanlockSetupLockspace(void)
             goto error;
         }
 
-        if (driver->group != -1)
+        if (driver->group != (gid_t) -1)
             perms |= 0060;
 
         if ((fd = open(path, O_WRONLY|O_CREAT|O_EXCL, perms)) < 0) {
@@ -249,7 +249,7 @@ static int virLockManagerSanlockSetupLockspace(void)
             VIR_DEBUG("Someone else just created lockspace %s", path);
         } else {
             /* chown() the path to make sure sanlock can access it */
-            if ((driver->user != -1 || driver->group != -1) &&
+            if ((driver->user != (uid_t) -1 || driver->group != (gid_t) -1) &&
                 (fchown(fd, driver->user, driver->group) < 0)) {
                 virReportSystemError(errno,
                                      _("cannot chown '%s' to (%u, %u)"),
@@ -303,8 +303,8 @@ static int virLockManagerSanlockSetupLockspace(void)
         }
     } else if (S_ISREG(st.st_mode)) {
         /* okay, the lease file exists. Check the permissions */
-        if (((driver->user != -1 && driver->user != st.st_uid) ||
-             (driver->group != -1 && driver->group != st.st_gid)) &&
+        if (((driver->user != (uid_t) -1 && driver->user != st.st_uid) ||
+             (driver->group != (gid_t) -1 && driver->group != st.st_gid)) &&
             (chown(path, driver->user, driver->group) < 0)) {
             virReportSystemError(errno,
                                  _("cannot chown '%s' to (%u, %u)"),
@@ -314,7 +314,7 @@ static int virLockManagerSanlockSetupLockspace(void)
             goto error;
         }
 
-        if ((driver->group != -1 && (st.st_mode & 0060) != 0060) &&
+        if ((driver->group != (gid_t) -1 && (st.st_mode & 0060) != 0060) &&
             chmod(path, 0660) < 0) {
             virReportSystemError(errno,
                                  _("cannot chmod '%s' to 0660"),
@@ -397,7 +397,8 @@ static int virLockManagerSanlockInit(unsigned int version,
     driver->requireLeaseForDisks = true;
     driver->hostID = 0;
     driver->autoDiskLease = false;
-    driver->user = driver->group = -1;
+    driver->user = (uid_t) -1;
+    driver->group = (gid_t) -1;
     if (!(driver->autoDiskLeasePath = strdup(LOCALSTATEDIR "/lib/libvirt/sanlock"))) {
         VIR_FREE(driver);
         virReportOOMError();
@@ -680,7 +681,7 @@ static int virLockManagerSanlockCreateLease(struct sanlk_resource *res)
             VIR_DEBUG("Someone else just created lockspace %s", res->disks[0].path);
         } else {
             /* chown() the path to make sure sanlock can access it */
-            if ((driver->user != -1 || driver->group != -1) &&
+            if ((driver->user != (uid_t) -1 || driver->group != (gid_t) -1) &&
                 (fchown(fd, driver->user, driver->group) < 0)) {
                 virReportSystemError(errno,
                                      _("cannot chown '%s' to (%u, %u)"),