lxclock: improve file locking

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

diff --git a/src/lxc/lxclock.c b/src/lxc/lxclock.c
index 8f533fa51805b9e77682d8b3d56c43318311506e..62fe2127f4e6a7bbf227d3b8d24127ca9481e176 100644 (file)
--- a/src/lxc/lxclock.c
+++ b/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,7 +178,7 @@ struct lxc_lock *lxc_newlock(const char *lxcpath, const char *name)
                        l = NULL;
                }
 
-               goto out;
+               goto on_error;
        }
 
        l->type = LXC_LOCK_FLOCK;
@@ -185,19 +186,19 @@ struct lxc_lock *lxc_newlock(const char *lxcpath, const char *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,9 +209,10 @@ 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;
@@ -218,25 +220,26 @@ int lxclock(struct lxc_lock *l, int timeout)
                        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;
        }