lockfile: Be more forgiving about lockfiles for symlinks.

As the database is being transitioned from /etc to /var, there is a symlink
from the old to the new location for the database and a symlink for its
lockfile.  This works OK, but it would be more user-friendly to still work
correctly in case the symlink for the lockfile isn't there (since its
existence is non-obvious), so this commit implements that behavior.

Signed-off-by: Ben Pfaff <blp@nicira.com>

diff --git a/lib/lockfile.c b/lib/lockfile.c
index c55be66..7ac3465 100644 (file)
--- a/lib/lockfile.c
+++ b/lib/lockfile.c
@@ -59,16 +59,29 @@ static int lockfile_try_lock(const char *name, bool block,
                              struct lockfile **lockfilep);
 
 /* Returns the name of the lockfile that would be created for locking a file
- * named 'file_name'.  The caller is responsible for freeing the returned
- * name, with free(), when it is no longer needed. */
+ * named 'filename_'.  The caller is responsible for freeing the returned name,
+ * with free(), when it is no longer needed. */
 char *
-lockfile_name(const char *file_name)
+lockfile_name(const char *filename_)
 {
-    const char *slash = strrchr(file_name, '/');
-    return (slash
-            ? xasprintf("%.*s/.%s.~lock~",
-                        (int) (slash - file_name), file_name, slash + 1)
-            : xasprintf(".%s.~lock~", file_name));
+    char *filename;
+    const char *slash;
+    char *lockname;
+
+    /* If 'filename_' is a symlink, base the name of the lockfile on the
+     * symlink's target rather than the name of the symlink.  That way, if a
+     * file is symlinked, but there is no symlink for its lockfile, then there
+     * is only a single lockfile for both the source and the target of the
+     * symlink, not one for each. */
+    filename = follow_symlinks(filename_);
+    slash = strrchr(filename, '/');
+    lockname = (slash
+                ? xasprintf("%.*s/.%s.~lock~",
+                            (int) (slash - filename), filename, slash + 1)
+                : xasprintf(".%s.~lock~", filename));
+    free(filename);
+
+    return lockname;
 }
 
 /* Locks the configuration file against modification by other processes and

diff --git a/tests/lockfile.at b/tests/lockfile.at
index 1fa0342..602cfab 100644 (file)
--- a/tests/lockfile.at
+++ b/tests/lockfile.at
@@ -19,3 +19,4 @@ CHECK_LOCKFILE([lock_timeout_gets_the_lock], [1])
 CHECK_LOCKFILE([lock_timeout_runs_out], [1])
 CHECK_LOCKFILE([lock_multiple], [0])
 CHECK_LOCKFILE([lock_symlink], [0])
+CHECK_LOCKFILE([lock_symlink_to_dir], [0])

diff --git a/tests/test-lockfile.c b/tests/test-lockfile.c
index 808ed1e..412010b 100644 (file)
--- a/tests/test-lockfile.c
+++ b/tests/test-lockfile.c
@@ -249,6 +249,35 @@ run_lock_symlink(void)
     CHECK(S_ISREG(s.st_mode) != 0, 1);
 }
 
+/* Checks that locking a file that is itself a symlink yields a lockfile in the
+ * directory that the symlink points to, named for the target of the
+ * symlink.
+ *
+ * (That is, if "a" is a symlink to "dir/b", then "a"'s lockfile is named
+ * "dir/.b.~lock".) */
+static void
+run_lock_symlink_to_dir(void)
+{
+    struct lockfile *a, *dummy;
+    struct stat s;
+
+    /* Create a symlink "a" pointing to "dir/b". */
+    CHECK(mkdir("dir", 0700), 0);
+    CHECK(symlink("dir/b", "a"), 0);
+    CHECK(lstat("a", &s), 0);
+    CHECK(S_ISLNK(s.st_mode) != 0, 1);
+
+    /* Lock 'a'. */
+    CHECK(lockfile_lock("a", 0, &a), 0);
+    CHECK(lstat("dir/.b.~lock~", &s), 0);
+    CHECK(S_ISREG(s.st_mode) != 0, 1);
+    CHECK(lstat(".a.~lock~", &s), -1);
+    CHECK(errno, ENOENT);
+    CHECK(lockfile_lock("dir/b", 0, &dummy), EDEADLK);
+
+    lockfile_unlock(a);
+}
+
 static void
 run_help(void)
 {
@@ -275,6 +304,7 @@ static const struct test tests[] = {
     TEST(lock_timeout_runs_out),
     TEST(lock_multiple),
     TEST(lock_symlink),
+    TEST(lock_symlink_to_dir),
     TEST(help),
     { NULL, NULL }
 #undef TEST