From b1fdc5fb270c0ca21de4bbe5002aca4dd79e9911 Mon Sep 17 00:00:00 2001 From: Ben Pfaff Date: Mon, 30 Jul 2012 14:41:13 -0700 Subject: [PATCH] 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 --- lib/lockfile.c | 29 +++++++++++++++++++++-------- tests/lockfile.at | 1 + tests/test-lockfile.c | 30 ++++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 8 deletions(-) diff --git a/lib/lockfile.c b/lib/lockfile.c index c55be661..7ac34659 100644 --- 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 1fa0342f..602cfab4 100644 --- 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 808ed1e3..412010b3 100644 --- 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 -- 2.30.2