lxclock: improve file locking

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

src/lxc/lxclock.c

@@ -96,9 +96,8 @@ static void unlock_mutex(pthread_mutex_t *l)
 static char *lxclock_name(const char *p, const char *n)
 {
 	int ret;
-	int len;
-	char *dest;
-	char *rundir;
+	size_t len;
+	char *dest, *rundir;
 
 	/* lockfile will be:
 	 * "/run" + "/lxc/lock/$lxcpath/$lxcname + '\0' if root
@@ -107,20 +106,22 @@ static char *lxclock_name(const char *p, const char *n)
 	 */
 
 	/* length of "/lxc/lock/" + $lxcpath + "/" + "." + $lxcname + '\0' */
-	len = strlen("/lxc/lock/") + strlen(n) + strlen(p) + 3;
+	len = (sizeof("/lxc/lock/") - 1) + strlen(n) + strlen(p) + 3;
+
 	rundir = get_rundir();
 	if (!rundir)
 		return NULL;
 
 	len += strlen(rundir);
 
-	if ((dest = malloc(len)) == NULL) {
+	dest = malloc(len);
+	if (!dest) {
 		free(rundir);
 		return NULL;
 	}
 
 	ret = snprintf(dest, len, "%s/lxc/lock/%s", rundir, p);
-	if (ret < 0 || ret >= len) {
+	if (ret < 0 || (size_t)ret >= len) {
 		free(dest);
 		free(rundir);
 		return NULL;
@@ -135,7 +136,7 @@ static char *lxclock_name(const char *p, const char *n)
 
 	ret = snprintf(dest, len, "%s/lxc/lock/%s/.%s", rundir, p, n);
 	free(rundir);
-	if (ret < 0 || ret >= len) {
+	if (ret < 0 || (size_t)ret >= len) {
 		free(dest);
 		return NULL;
 	}
@@ -145,15 +146,15 @@ static char *lxclock_name(const char *p, const char *n)
 
 static sem_t *lxc_new_unnamed_sem(void)
 {
-	sem_t *s;
 	int ret;
+	sem_t *s;
 
 	s = malloc(sizeof(*s));
 	if (!s)
 		return NULL;
 
 	ret = sem_init(s, 0, 1);
-	if (ret) {
+	if (ret < 0) {
 		free(s);
 		return NULL;
 	}
@@ -167,7 +168,7 @@ struct lxc_lock *lxc_newlock(const char *lxcpath, const char *name)
 
 	l = malloc(sizeof(*l));
 	if (!l)
-		goto out;
+		goto on_error;
 
 	if (!name) {
 		l->type = LXC_LOCK_ANON_SEM;
@@ -177,27 +178,27 @@ struct lxc_lock *lxc_newlock(const char *lxcpath, const char *name)
 			l = NULL;
 		}
 
-		goto out;
+		goto on_error;
 	}
 
 	l->type = LXC_LOCK_FLOCK;
 	l->u.f.fname = lxclock_name(lxcpath, name);
 	if (!l->u.f.fname) {
 		free(l);
 		l = NULL;
-		goto out;
+		goto on_error;
 	}
 
 	l->u.f.fd = -1;
 
-out:
+on_error:
 	return l;
 }
 
 int lxclock(struct lxc_lock *l, int timeout)
 {
-	int ret = -1, saved_errno = errno;
 	struct flock lk;
+	int ret = -1, saved_errno = errno;
 
 	switch(l->type) {
 	case LXC_LOCK_ANON_SEM:
@@ -208,35 +209,37 @@ int lxclock(struct lxc_lock *l, int timeout)
 		} else {
 			struct timespec ts;
 
-			if (clock_gettime(CLOCK_REALTIME, &ts) == -1) {
+			ret = clock_gettime(CLOCK_REALTIME, &ts);
+			if (ret < 0) {
 				ret = -2;
-				goto out;
+				goto on_error;
 			}
 
 			ts.tv_sec += timeout;
 			ret = sem_timedwait(l->u.sem, &ts);
 			if (ret < 0)
 				saved_errno = errno;
 		}
+
 		break;
 	case LXC_LOCK_FLOCK:
 		ret = -2;
 		if (timeout) {
-			ERROR("Error: timeout not supported with flock");
-			goto out;
+			ERROR("Timeouts are not supported with file locks");
+			goto on_error;
 		}
 
 		if (!l->u.f.fname) {
-			ERROR("Error: filename not set for flock");
-			goto out;
+			ERROR("No filename set for file lock");
+			goto on_error;
 		}
 
 		if (l->u.f.fd == -1) {
 			l->u.f.fd = open(l->u.f.fname, O_CREAT | O_RDWR | O_NOFOLLOW | O_CLOEXEC | O_NOCTTY, S_IWUSR | S_IRUSR);
 			if (l->u.f.fd == -1) {
-				ERROR("Error opening %s", l->u.f.fname);
+				SYSERROR("Failed to open \"%s\"", l->u.f.fname);
 				saved_errno = errno;
-				goto out;
+				goto on_error;
 			}
 		}
 
@@ -251,27 +254,29 @@ int lxclock(struct lxc_lock *l, int timeout)
 				ret = flock(l->u.f.fd, LOCK_EX);
 			saved_errno = errno;
 		}
+
 		break;
 	}
 
-out:
+on_error:
 	errno = saved_errno;
 	return ret;
 }
 
 int lxcunlock(struct lxc_lock *l)
 {
-	int ret = 0, saved_errno = errno;
 	struct flock lk;
+	int ret = 0, saved_errno = errno;
 
-	switch(l->type) {
+	switch (l->type) {
 	case LXC_LOCK_ANON_SEM:
-		if (!l->u.sem)
+		if (!l->u.sem) {
 			ret = -2;
-		else {
+		} else {
 			ret = sem_post(l->u.sem);
 			saved_errno = errno;
 		}
+
 		break;
 	case LXC_LOCK_FLOCK:
 		if (l->u.f.fd != -1) {
@@ -289,8 +294,10 @@ int lxcunlock(struct lxc_lock *l)
 
 			close(l->u.f.fd);
 			l->u.f.fd = -1;
-		} else
+		} else {
 			ret = -2;
+		}
+
 		break;
 	}
 
@@ -317,6 +324,7 @@ void lxc_putlock(struct lxc_lock *l)
 			free(l->u.sem);
 			l->u.sem = NULL;
 		}
+
 		break;
 	case LXC_LOCK_FLOCK:
 		if (l->u.f.fd != -1) {
@@ -326,8 +334,10 @@ void lxc_putlock(struct lxc_lock *l)
 
 		free(l->u.f.fname);
 		l->u.f.fname = NULL;
+
 		break;
 	}
+
 	free(l);
 }
 
@@ -355,10 +365,12 @@ int container_disk_lock(struct lxc_container *c)
 {
 	int ret;
 
-	if ((ret = lxclock(c->privlock, 0)))
+	ret = lxclock(c->privlock, 0);
+	if (ret < 0)
 		return ret;
 
-	if ((ret = lxclock(c->slock, 0))) {
+	ret = lxclock(c->slock, 0);
+	if (ret < 0) {
 		lxcunlock(c->privlock);
 		return ret;
 	}