[libvirt] [PATCH] maint: avoid potential promotion issues with [ug]id_t
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 | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/locking/lock_driver_sanlock.c b/src/locking/lock_driver_sanlock.c
index c955003..d06fa66 100644
--- a/src/locking/lock_driver_sanlock.c
+++ b/src/locking/lock_driver_sanlock.c
@@ -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)"),
--
1.8.0.2