summaryrefslogtreecommitdiff
path: root/read-cache.c
diff options
context:
space:
mode:
authorMichael Haggerty <mhagger@alum.mit.edu>2014-10-01 12:28:27 +0200
committerJunio C Hamano <gitster@pobox.com>2014-10-01 13:48:59 -0700
commit707103fdfd0c03511fa547d9b80638d8160f1a88 (patch)
tree8d2a349374c972fb0486d08b0fccea4d771bc451 /read-cache.c
parente831855ecc6783bfe4b681017349c623fc2fe8c8 (diff)
downloadgit-707103fdfd0c03511fa547d9b80638d8160f1a88.tar.gz
lockfile: avoid transitory invalid states
Because remove_lock_file() can be called any time by the signal handler, it is important that any lock_file objects that are in the lock_file_list are always in a valid state. And since lock_file objects are often reused (but are never removed from lock_file_list), that means we have to be careful whenever mutating a lock_file object to always keep it in a well-defined state. This was formerly not the case, because part of the state was encoded by setting lk->filename to the empty string vs. a valid filename. It is wrong to assume that this string can be updated atomically; for example, even strcpy(lk->filename, value) is unsafe. But the old code was even more reckless; for example, strcpy(lk->filename, path); if (!(flags & LOCK_NODEREF)) resolve_symlink(lk->filename, max_path_len); strcat(lk->filename, ".lock"); During the call to resolve_symlink(), lk->filename contained the name of the file that was being locked, not the name of the lockfile. If a signal were raised during that interval, then the signal handler would have deleted the valuable file! We could probably continue to use the filename field to encode the state by being careful to write characters 1..N-1 of the filename first, and then overwrite the NUL at filename[0] with the first character of the filename, but that would be awkward and error-prone. So, instead of using the filename field to determine whether the lock_file object is active, add a new field "lock_file::active" for this purpose. Be careful to set this field only when filename really contains the name of a file that should be deleted on cleanup. Helped-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'read-cache.c')
-rw-r--r--read-cache.c1
1 files changed, 1 insertions, 0 deletions
diff --git a/read-cache.c b/read-cache.c
index 5ffb1d7bb6..af69f344a2 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -2046,6 +2046,7 @@ static int commit_locked_index(struct lock_file *lk)
return -1;
if (rename(lk->filename, alternate_index_output))
return -1;
+ lk->active = 0;
lk->filename[0] = 0;
return 0;
} else {