From e197c21807dacadc8305250baa0b9228819189d4 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 1 Oct 2014 12:28:05 +0200 Subject: unable_to_lock_die(): rename function from unable_to_lock_index_die() This function is used for other things besides the index, so rename it accordingly. Suggested-by: Jeff King Signed-off-by: Michael Haggerty Reviewed-by: Ronnie Sahlberg Reviewed-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- cache.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'cache.h') diff --git a/cache.h b/cache.h index 8206039cfe..0a76d023e4 100644 --- a/cache.h +++ b/cache.h @@ -582,7 +582,7 @@ struct lock_file { extern int unable_to_lock_error(const char *path, int err); extern void unable_to_lock_message(const char *path, int err, struct strbuf *buf); -extern NORETURN void unable_to_lock_index_die(const char *path, int err); +extern NORETURN void unable_to_lock_die(const char *path, int err); extern int hold_lock_file_for_update(struct lock_file *, const char *path, int); extern int hold_lock_file_for_append(struct lock_file *, const char *path, int); extern int commit_lock_file(struct lock_file *); -- cgit v1.3-5-g9baa From 7108ad232fc7a4c889e82b40c52125adc9796ff5 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 1 Oct 2014 12:28:15 +0200 Subject: cache.h: define constants LOCK_SUFFIX and LOCK_SUFFIX_LEN There are a few places that use these values, so define constants for them. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- cache.h | 4 ++++ lockfile.c | 11 ++++++----- refs.c | 7 ++++--- 3 files changed, 14 insertions(+), 8 deletions(-) (limited to 'cache.h') diff --git a/cache.h b/cache.h index 0a76d023e4..24891a81d6 100644 --- a/cache.h +++ b/cache.h @@ -570,6 +570,10 @@ extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st); #define REFRESH_IN_PORCELAIN 0x0020 /* user friendly output, not "needs update" */ extern int refresh_index(struct index_state *, unsigned int flags, const struct pathspec *pathspec, char *seen, const char *header_msg); +/* String appended to a filename to derive the lockfile name: */ +#define LOCK_SUFFIX ".lock" +#define LOCK_SUFFIX_LEN 5 + struct lock_file { struct lock_file *next; int fd; diff --git a/lockfile.c b/lockfile.c index 2680dc949d..23847fc9f2 100644 --- a/lockfile.c +++ b/lockfile.c @@ -166,10 +166,11 @@ static char *resolve_symlink(char *p, size_t s) static int lock_file(struct lock_file *lk, const char *path, int flags) { /* - * subtract 5 from size to make sure there's room for adding - * ".lock" for the lock file name + * subtract LOCK_SUFFIX_LEN from size to make sure there's + * room for adding ".lock" for the lock file name: */ - static const size_t max_path_len = sizeof(lk->filename) - 5; + static const size_t max_path_len = sizeof(lk->filename) - + LOCK_SUFFIX_LEN; if (!lock_file_list) { /* One-time initialization */ @@ -194,7 +195,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags) strcpy(lk->filename, path); if (!(flags & LOCK_NODEREF)) resolve_symlink(lk->filename, max_path_len); - strcat(lk->filename, ".lock"); + strcat(lk->filename, LOCK_SUFFIX); lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666); if (0 <= lk->fd) { lk->owner = getpid(); @@ -308,7 +309,7 @@ int commit_lock_file(struct lock_file *lk) if (close_lock_file(lk)) return -1; strcpy(result_file, lk->filename); - i = strlen(result_file) - 5; /* .lock */ + i = strlen(result_file) - LOCK_SUFFIX_LEN; /* .lock */ result_file[i] = 0; if (rename(lk->filename, result_file)) return -1; diff --git a/refs.c b/refs.c index 0e324770c9..73d6baedb0 100644 --- a/refs.c +++ b/refs.c @@ -79,7 +79,8 @@ out: if (refname[1] == '\0') return -1; /* Component equals ".". */ } - if (cp - refname >= 5 && !memcmp(cp - 5, ".lock", 5)) + if (cp - refname >= LOCK_SUFFIX_LEN && + !memcmp(cp - LOCK_SUFFIX_LEN, LOCK_SUFFIX, LOCK_SUFFIX_LEN)) return -1; /* Refname ends with ".lock". */ return cp - refname; } @@ -2602,11 +2603,11 @@ static int delete_ref_loose(struct ref_lock *lock, int flag) { if (!(flag & REF_ISPACKED) || flag & REF_ISSYMREF) { /* loose */ - int err, i = strlen(lock->lk->filename) - 5; /* .lock */ + int err, i = strlen(lock->lk->filename) - LOCK_SUFFIX_LEN; lock->lk->filename[i] = 0; err = unlink_or_warn(lock->lk->filename); - lock->lk->filename[i] = '.'; + lock->lk->filename[i] = LOCK_SUFFIX[0]; if (err && errno != ENOENT) return 1; } -- cgit v1.3-5-g9baa From 707103fdfd0c03511fa547d9b80638d8160f1a88 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 1 Oct 2014 12:28:27 +0200 Subject: 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 Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- cache.h | 1 + lockfile.c | 37 ++++++++++++++++++++++++++----------- read-cache.c | 1 + 3 files changed, 28 insertions(+), 11 deletions(-) (limited to 'cache.h') diff --git a/cache.h b/cache.h index 24891a81d6..8e25fce3d8 100644 --- a/cache.h +++ b/cache.h @@ -576,6 +576,7 @@ extern int refresh_index(struct index_state *, unsigned int flags, const struct struct lock_file { struct lock_file *next; + volatile sig_atomic_t active; int fd; pid_t owner; char on_list; diff --git a/lockfile.c b/lockfile.c index 728ce49765..d35ac448f0 100644 --- a/lockfile.c +++ b/lockfile.c @@ -23,7 +23,7 @@ * used to prevent a forked process from closing a lockfile created by * its parent. * - * A lock_file object can be in several states: + * The possible states of a lock_file object are as follows: * * - Uninitialized. In this state the object's on_list field must be * zero but the rest of its contents need not be initialized. As @@ -32,19 +32,27 @@ * * - Locked, lockfile open (after hold_lock_file_for_update(), * hold_lock_file_for_append(), or reopen_lock_file()). In this - * state, the lockfile exists, filename holds the filename of the - * lockfile, fd holds a file descriptor open for writing to the - * lockfile, and owner holds the PID of the process that locked the - * file. + * state: + * - the lockfile exists + * - active is set + * - filename holds the filename of the lockfile + * - fd holds a file descriptor open for writing to the lockfile + * - owner holds the PID of the process that locked the file * * - Locked, lockfile closed (after successful close_lock_file()). * Same as the previous state, except that the lockfile is closed * and fd is -1. * * - Unlocked (after commit_lock_file(), rollback_lock_file(), a - * failed attempt to lock, or a failed close_lock_file()). In this - * state, filename[0] == '\0' and fd is -1. The object is left - * registered in the lock_file_list, and on_list is set. + * failed attempt to lock, or a failed close_lock_file()). In this + * state: + * - active is unset + * - filename[0] == '\0' (usually, though there are transitory states + * in which this condition doesn't hold). Client code should *not* + * rely on this fact! + * - fd is -1 + * - the object is left registered in the lock_file_list, and + * on_list is set. */ static struct lock_file *lock_file_list; @@ -175,9 +183,13 @@ static int lock_file(struct lock_file *lk, const char *path, int flags) atexit(remove_lock_file); } + if (lk->active) + die("BUG: cannot lock_file(\"%s\") using active struct lock_file", + path); if (!lk->on_list) { /* Initialize *lk and add it to lock_file_list: */ lk->fd = -1; + lk->active = 0; lk->owner = 0; lk->filename[0] = 0; lk->next = lock_file_list; @@ -199,6 +211,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags) return -1; } lk->owner = getpid(); + lk->active = 1; if (adjust_shared_perm(lk->filename)) { int save_errno = errno; error("cannot fix permission bits on %s", lk->filename); @@ -298,7 +311,7 @@ int reopen_lock_file(struct lock_file *lk) { if (0 <= lk->fd) die(_("BUG: reopen a lockfile that is still open")); - if (!lk->filename[0]) + if (!lk->active) die(_("BUG: reopen a lockfile that has been committed")); lk->fd = open(lk->filename, O_WRONLY); return lk->fd; @@ -308,7 +321,7 @@ int commit_lock_file(struct lock_file *lk) { char result_file[PATH_MAX]; - if (!lk->filename[0]) + if (!lk->active) die("BUG: attempt to commit unlocked object"); if (close_lock_file(lk)) @@ -325,6 +338,7 @@ int commit_lock_file(struct lock_file *lk) return -1; } + lk->active = 0; lk->filename[0] = 0; return 0; } @@ -339,11 +353,12 @@ int hold_locked_index(struct lock_file *lk, int die_on_error) void rollback_lock_file(struct lock_file *lk) { - if (!lk->filename[0]) + if (!lk->active) return; if (!close_lock_file(lk)) { unlink_or_warn(lk->filename); + lk->active = 0; lk->filename[0] = 0; } } 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 { -- cgit v1.3-5-g9baa From 2091c5062c6fd928e1ad3e7c059243e597cb8bbf Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 1 Oct 2014 12:28:28 +0200 Subject: struct lock_file: declare some fields volatile The function remove_lock_file_on_signal() is used as a signal handler. It is not realistic to make the signal handler conform strictly to the C standard, which is very restrictive about what a signal handler is allowed to do. But let's increase the likelihood that it will work: The lock_file_list global variable and several fields from struct lock_file are used by the signal handler. Declare those values "volatile" to (1) force the main process to write the values to RAM promptly, and (2) prevent updates to these fields from being reordered in a way that leaves an opportunity for a jump to the signal handler while the object is in an inconsistent state. We don't mark the filename field volatile because that would prevent the use of strcpy(), and it is anyway unlikely that a compiler re-orders a strcpy() call across other expressions. So in practice it should be possible to get away without "volatile" in the "filename" case. Suggested-by: Johannes Sixt Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- cache.h | 6 +++--- lockfile.c | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'cache.h') diff --git a/cache.h b/cache.h index 8e25fce3d8..c2ea6f15e6 100644 --- a/cache.h +++ b/cache.h @@ -575,10 +575,10 @@ extern int refresh_index(struct index_state *, unsigned int flags, const struct #define LOCK_SUFFIX_LEN 5 struct lock_file { - struct lock_file *next; + struct lock_file *volatile next; volatile sig_atomic_t active; - int fd; - pid_t owner; + volatile int fd; + volatile pid_t owner; char on_list; char filename[PATH_MAX]; }; diff --git a/lockfile.c b/lockfile.c index d35ac448f0..89043f5acc 100644 --- a/lockfile.c +++ b/lockfile.c @@ -55,7 +55,7 @@ * on_list is set. */ -static struct lock_file *lock_file_list; +static struct lock_file *volatile lock_file_list; static void remove_lock_file(void) { -- cgit v1.3-5-g9baa From cf6950d3bfe1447ac04867b1f5654a2fc9c5db96 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 1 Oct 2014 12:28:32 +0200 Subject: lockfile: change lock_file::filename into a strbuf MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For now, we still make sure to allocate at least PATH_MAX characters for the strbuf because resolve_symlink() doesn't know how to expand the space for its return value. (That will be fixed in a moment.) Another alternative would be to just use a strbuf as scratch space in lock_file() but then store a pointer to the naked string in struct lock_file. But lock_file objects are often reused. By reusing the same strbuf, we can avoid having to reallocate the string most times when a lock_file object is reused. Helped-by: Torsten Bögershausen Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/commit.c | 12 ++++++------ builtin/reflog.c | 2 +- cache.h | 2 +- config.c | 14 +++++++------- lockfile.c | 53 ++++++++++++++++++++++++----------------------------- read-cache.c | 4 ++-- refs.c | 6 +++--- shallow.c | 6 +++--- 8 files changed, 47 insertions(+), 52 deletions(-) (limited to 'cache.h') diff --git a/builtin/commit.c b/builtin/commit.c index 70f5935f1b..f55e80986b 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -341,7 +341,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix die(_("unable to create temporary index")); old_index_env = getenv(INDEX_ENVIRONMENT); - setenv(INDEX_ENVIRONMENT, index_lock.filename, 1); + setenv(INDEX_ENVIRONMENT, index_lock.filename.buf, 1); if (interactive_add(argc, argv, prefix, patch_interactive) != 0) die(_("interactive add failed")); @@ -352,7 +352,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix unsetenv(INDEX_ENVIRONMENT); discard_cache(); - read_cache_from(index_lock.filename); + read_cache_from(index_lock.filename.buf); if (update_main_cache_tree(WRITE_TREE_SILENT) == 0) { if (reopen_lock_file(&index_lock) < 0) die(_("unable to write index file")); @@ -362,7 +362,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix warning(_("Failed to update main cache tree")); commit_style = COMMIT_NORMAL; - return index_lock.filename; + return index_lock.filename.buf; } /* @@ -385,7 +385,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix if (write_locked_index(&the_index, &index_lock, CLOSE_LOCK)) die(_("unable to write new_index file")); commit_style = COMMIT_NORMAL; - return index_lock.filename; + return index_lock.filename.buf; } /* @@ -472,9 +472,9 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix die(_("unable to write temporary index file")); discard_cache(); - read_cache_from(false_lock.filename); + read_cache_from(false_lock.filename.buf); - return false_lock.filename; + return false_lock.filename.buf; } static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn, diff --git a/builtin/reflog.c b/builtin/reflog.c index e8a8fb13b9..7c78b15c14 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -431,7 +431,7 @@ static int expire_reflog(const char *ref, const unsigned char *sha1, int unused, write_str_in_full(lock->lock_fd, "\n") != 1 || close_ref(lock) < 0)) { status |= error("Couldn't write %s", - lock->lk->filename); + lock->lk->filename.buf); unlink(newlog_path); } else if (rename(newlog_path, log_file)) { status |= error("cannot rename %s to %s", diff --git a/cache.h b/cache.h index c2ea6f15e6..f81d95fc3c 100644 --- a/cache.h +++ b/cache.h @@ -580,7 +580,7 @@ struct lock_file { volatile int fd; volatile pid_t owner; char on_list; - char filename[PATH_MAX]; + struct strbuf filename; }; #define LOCK_DIE_ON_ERROR 1 #define LOCK_NODEREF 2 diff --git a/config.c b/config.c index 123ed29963..21107797d3 100644 --- a/config.c +++ b/config.c @@ -2024,9 +2024,9 @@ int git_config_set_multivar_in_file(const char *config_filename, MAP_PRIVATE, in_fd, 0); close(in_fd); - if (chmod(lock->filename, st.st_mode & 07777) < 0) { + if (chmod(lock->filename.buf, st.st_mode & 07777) < 0) { error("chmod on %s failed: %s", - lock->filename, strerror(errno)); + lock->filename.buf, strerror(errno)); ret = CONFIG_NO_WRITE; goto out_free; } @@ -2106,7 +2106,7 @@ out_free: return ret; write_err_out: - ret = write_error(lock->filename); + ret = write_error(lock->filename.buf); goto out_free; } @@ -2207,9 +2207,9 @@ int git_config_rename_section_in_file(const char *config_filename, fstat(fileno(config_file), &st); - if (chmod(lock->filename, st.st_mode & 07777) < 0) { + if (chmod(lock->filename.buf, st.st_mode & 07777) < 0) { ret = error("chmod on %s failed: %s", - lock->filename, strerror(errno)); + lock->filename.buf, strerror(errno)); goto out; } @@ -2230,7 +2230,7 @@ int git_config_rename_section_in_file(const char *config_filename, } store.baselen = strlen(new_name); if (!store_write_section(out_fd, new_name)) { - ret = write_error(lock->filename); + ret = write_error(lock->filename.buf); goto out; } /* @@ -2256,7 +2256,7 @@ int git_config_rename_section_in_file(const char *config_filename, continue; length = strlen(output); if (write_in_full(out_fd, output, length) != length) { - ret = write_error(lock->filename); + ret = write_error(lock->filename.buf); goto out; } } diff --git a/lockfile.c b/lockfile.c index 1dd118f556..85c8648c51 100644 --- a/lockfile.c +++ b/lockfile.c @@ -47,9 +47,9 @@ * failed attempt to lock, or a failed close_lock_file()). In this * state: * - active is unset - * - filename[0] == '\0' (usually, though there are transitory states - * in which this condition doesn't hold). Client code should *not* - * rely on this fact! + * - filename is empty (usually, though there are transitory + * states in which this condition doesn't hold). Client code should + * *not* rely on the filename being empty in this state. * - fd is -1 * - the object is left registered in the lock_file_list, and * on_list is set. @@ -170,13 +170,6 @@ static char *resolve_symlink(char *p, size_t s) /* Make sure errno contains a meaningful value on error */ static int lock_file(struct lock_file *lk, const char *path, int flags) { - /* - * subtract LOCK_SUFFIX_LEN from size to make sure there's - * room for adding ".lock" for the lock file name: - */ - static const size_t max_path_len = sizeof(lk->filename) - - LOCK_SUFFIX_LEN; - if (!lock_file_list) { /* One-time initialization */ sigchain_push_common(remove_lock_file_on_signal); @@ -191,30 +184,32 @@ static int lock_file(struct lock_file *lk, const char *path, int flags) lk->fd = -1; lk->active = 0; lk->owner = 0; - lk->filename[0] = 0; + strbuf_init(&lk->filename, PATH_MAX); lk->next = lock_file_list; lock_file_list = lk; lk->on_list = 1; + } else if (lk->filename.len) { + /* This shouldn't happen, but better safe than sorry. */ + die("BUG: lock_file(\"%s\") called with improperly-reset lock_file object", + path); } - if (strlen(path) >= max_path_len) { - errno = ENAMETOOLONG; - return -1; + strbuf_addstr(&lk->filename, path); + if (!(flags & LOCK_NODEREF)) { + resolve_symlink(lk->filename.buf, lk->filename.alloc); + strbuf_setlen(&lk->filename, strlen(lk->filename.buf)); } - strcpy(lk->filename, path); - if (!(flags & LOCK_NODEREF)) - resolve_symlink(lk->filename, max_path_len); - strcat(lk->filename, LOCK_SUFFIX); - lk->fd = open(lk->filename, O_RDWR | O_CREAT | O_EXCL, 0666); + strbuf_addstr(&lk->filename, LOCK_SUFFIX); + lk->fd = open(lk->filename.buf, O_RDWR | O_CREAT | O_EXCL, 0666); if (lk->fd < 0) { - lk->filename[0] = 0; + strbuf_reset(&lk->filename); return -1; } lk->owner = getpid(); lk->active = 1; - if (adjust_shared_perm(lk->filename)) { + if (adjust_shared_perm(lk->filename.buf)) { int save_errno = errno; - error("cannot fix permission bits on %s", lk->filename); + error("cannot fix permission bits on %s", lk->filename.buf); rollback_lock_file(lk); errno = save_errno; return -1; @@ -313,7 +308,7 @@ int reopen_lock_file(struct lock_file *lk) die(_("BUG: reopen a lockfile that is still open")); if (!lk->active) die(_("BUG: reopen a lockfile that has been committed")); - lk->fd = open(lk->filename, O_WRONLY); + lk->fd = open(lk->filename.buf, O_WRONLY); return lk->fd; } @@ -329,9 +324,9 @@ int commit_lock_file(struct lock_file *lk) return -1; /* remove ".lock": */ - strbuf_add(&result_file, lk->filename, - strlen(lk->filename) - LOCK_SUFFIX_LEN); - err = rename(lk->filename, result_file.buf); + strbuf_add(&result_file, lk->filename.buf, + lk->filename.len - LOCK_SUFFIX_LEN); + err = rename(lk->filename.buf, result_file.buf); strbuf_reset(&result_file); if (err) { int save_errno = errno; @@ -341,7 +336,7 @@ int commit_lock_file(struct lock_file *lk) } lk->active = 0; - lk->filename[0] = 0; + strbuf_reset(&lk->filename); return 0; } @@ -359,8 +354,8 @@ void rollback_lock_file(struct lock_file *lk) return; if (!close_lock_file(lk)) { - unlink_or_warn(lk->filename); + unlink_or_warn(lk->filename.buf); lk->active = 0; - lk->filename[0] = 0; + strbuf_reset(&lk->filename); } } diff --git a/read-cache.c b/read-cache.c index af69f344a2..91bf876ee6 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2044,10 +2044,10 @@ static int commit_locked_index(struct lock_file *lk) if (alternate_index_output) { if (close_lock_file(lk)) return -1; - if (rename(lk->filename, alternate_index_output)) + if (rename(lk->filename.buf, alternate_index_output)) return -1; lk->active = 0; - lk->filename[0] = 0; + strbuf_reset(&lk->filename); return 0; } else { return commit_lock_file(lk); diff --git a/refs.c b/refs.c index a4151315e3..598f4eb9e7 100644 --- a/refs.c +++ b/refs.c @@ -2607,8 +2607,8 @@ static int delete_ref_loose(struct ref_lock *lock, int flag) * lockfile name, minus ".lock": */ char *loose_filename = xmemdupz( - lock->lk->filename, - strlen(lock->lk->filename) - LOCK_SUFFIX_LEN); + lock->lk->filename.buf, + lock->lk->filename.len - LOCK_SUFFIX_LEN); int err = unlink_or_warn(loose_filename); free(loose_filename); if (err && errno != ENOENT) @@ -2972,7 +2972,7 @@ int write_ref_sha1(struct ref_lock *lock, write_in_full(lock->lock_fd, &term, 1) != 1 || close_ref(lock) < 0) { int save_errno = errno; - error("Couldn't write %s", lock->lk->filename); + error("Couldn't write %s", lock->lk->filename.buf); unlock_ref(lock); errno = save_errno; return -1; diff --git a/shallow.c b/shallow.c index 57f4afa6b4..4919baf772 100644 --- a/shallow.c +++ b/shallow.c @@ -269,8 +269,8 @@ void setup_alternate_shallow(struct lock_file *shallow_lock, if (write_shallow_commits(&sb, 0, extra)) { if (write_in_full(fd, sb.buf, sb.len) != sb.len) die_errno("failed to write to %s", - shallow_lock->filename); - *alternate_shallow_file = shallow_lock->filename; + shallow_lock->filename.buf); + *alternate_shallow_file = shallow_lock->filename.buf; } else /* * is_repository_shallow() sees empty string as "no @@ -316,7 +316,7 @@ void prune_shallow(int show_only) if (write_shallow_commits_1(&sb, 0, NULL, SEEN_ONLY)) { if (write_in_full(fd, sb.buf, sb.len) != sb.len) die_errno("failed to write to %s", - shallow_lock.filename); + shallow_lock.filename.buf); commit_lock_file(&shallow_lock); } else { unlink(git_path("shallow")); -- cgit v1.3-5-g9baa From 751bacedaa507b7b6d10b2c1f48e019a01a8fa6e Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 1 Oct 2014 12:28:36 +0200 Subject: commit_lock_file_to(): refactor a helper out of commit_lock_file() commit_locked_index(), when writing to an alternate index file, duplicates (poorly) the code in commit_lock_file(). And anyway, it shouldn't have to know so much about the internal workings of lockfile objects. So extract a new function commit_lock_file_to() that does the work common to the two functions, and call it from both commit_lock_file() and commit_locked_index(). Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- Documentation/technical/api-lockfile.txt | 34 ++++++++++++++++----------- cache.h | 1 + lockfile.c | 40 +++++++++++++++++++++----------- read-cache.c | 13 +++-------- 4 files changed, 50 insertions(+), 38 deletions(-) (limited to 'cache.h') diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt index 9805da099d..aa7d822900 100644 --- a/Documentation/technical/api-lockfile.txt +++ b/Documentation/technical/api-lockfile.txt @@ -49,14 +49,14 @@ The caller: When finished writing, the caller can: * Close the file descriptor and rename the lockfile to its final - destination by calling `commit_lock_file`. + destination by calling `commit_lock_file` or `commit_lock_file_to`. * Close the file descriptor and remove the lockfile by calling `rollback_lock_file`. * Close the file descriptor without removing or renaming the lockfile by calling `close_lock_file`, and later call `commit_lock_file`, - `rollback_lock_file`, or `reopen_lock_file`. + `commit_lock_file_to`, `rollback_lock_file`, or `reopen_lock_file`. Even after the lockfile is committed or rolled back, the `lock_file` object must not be freed or altered by the caller. However, it may be @@ -64,20 +64,19 @@ reused; just pass it to another call of `hold_lock_file_for_update` or `hold_lock_file_for_append`. If the program exits before you have called one of `commit_lock_file`, -`rollback_lock_file`, or `close_lock_file`, an `atexit(3)` handler -will close and remove the lockfile, rolling back any uncommitted -changes. +`commit_lock_file_to`, `rollback_lock_file`, or `close_lock_file`, an +`atexit(3)` handler will close and remove the lockfile, rolling back +any uncommitted changes. If you need to close the file descriptor you obtained from a `hold_lock_file_*` function yourself, do so by calling `close_lock_file`. You should never call `close(2)` yourself! Otherwise the `struct lock_file` structure would still think that the -file descriptor needs to be closed, and a later call to -`commit_lock_file` or `rollback_lock_file` or program exit would +file descriptor needs to be closed, and a commit or rollback would result in duplicate calls to `close(2)`. Worse yet, if you `close(2)` and then later open another file descriptor for a completely different -purpose, then a call to `commit_lock_file` or `rollback_lock_file` -might close that unrelated file descriptor. +purpose, then a commit or rollback might close that unrelated file +descriptor. Error handling @@ -100,9 +99,9 @@ unable_to_lock_die:: Emit an appropriate error message and `die()`. -Similarly, `commit_lock_file` and `close_lock_file` return 0 on -success. On failure they set `errno` appropriately, do their best to -roll back the lockfile, and return -1. +Similarly, `commit_lock_file`, `commit_lock_file_to`, and +`close_lock_file` return 0 on success. On failure they set `errno` +appropriately, do their best to roll back the lockfile, and return -1. Flags @@ -156,6 +155,12 @@ commit_lock_file:: `commit_lock_file` for a `lock_file` object that is not currently locked. +commit_lock_file_to:: + + Like `commit_lock_file()`, except that it takes an explicit + `path` argument to which the lockfile should be renamed. The + `path` must be on the same filesystem as the lock file. + rollback_lock_file:: Take a pointer to the `struct lock_file` initialized with an @@ -172,8 +177,9 @@ close_lock_file:: `hold_lock_file_for_append`, and close the file descriptor. Return 0 upon success. On failure to `close(2)`, return a negative value and roll back the lock file. Usually - `commit_lock_file` or `rollback_lock_file` should eventually - be called if `close_lock_file` succeeds. + `commit_lock_file`, `commit_lock_file_to`, or + `rollback_lock_file` should eventually be called if + `close_lock_file` succeeds. reopen_lock_file:: diff --git a/cache.h b/cache.h index f81d95fc3c..414e93ca6b 100644 --- a/cache.h +++ b/cache.h @@ -590,6 +590,7 @@ extern void unable_to_lock_message(const char *path, int err, extern NORETURN void unable_to_lock_die(const char *path, int err); extern int hold_lock_file_for_update(struct lock_file *, const char *path, int); extern int hold_lock_file_for_append(struct lock_file *, const char *path, int); +extern int commit_lock_file_to(struct lock_file *, const char *path); extern int commit_lock_file(struct lock_file *); extern int reopen_lock_file(struct lock_file *); extern void update_index_if_able(struct index_state *, struct lock_file *); diff --git a/lockfile.c b/lockfile.c index 56ad7e8a6e..cf7f4d0470 100644 --- a/lockfile.c +++ b/lockfile.c @@ -43,9 +43,9 @@ * Same as the previous state, except that the lockfile is closed * and fd is -1. * - * - Unlocked (after commit_lock_file(), rollback_lock_file(), a - * failed attempt to lock, or a failed close_lock_file()). In this - * state: + * - Unlocked (after commit_lock_file(), commit_lock_file_to(), + * rollback_lock_file(), a failed attempt to lock, or a failed + * close_lock_file()). In this state: * - active is unset * - filename is empty (usually, though there are transitory * states in which this condition doesn't hold). Client code should @@ -284,23 +284,15 @@ int reopen_lock_file(struct lock_file *lk) return lk->fd; } -int commit_lock_file(struct lock_file *lk) +int commit_lock_file_to(struct lock_file *lk, const char *path) { - static struct strbuf result_file = STRBUF_INIT; - int err; - if (!lk->active) - die("BUG: attempt to commit unlocked object"); + die("BUG: attempt to commit unlocked object to \"%s\"", path); if (close_lock_file(lk)) return -1; - /* remove ".lock": */ - strbuf_add(&result_file, lk->filename.buf, - lk->filename.len - LOCK_SUFFIX_LEN); - err = rename(lk->filename.buf, result_file.buf); - strbuf_reset(&result_file); - if (err) { + if (rename(lk->filename.buf, path)) { int save_errno = errno; rollback_lock_file(lk); errno = save_errno; @@ -312,6 +304,26 @@ int commit_lock_file(struct lock_file *lk) return 0; } +int commit_lock_file(struct lock_file *lk) +{ + static struct strbuf result_file = STRBUF_INIT; + int err; + + if (!lk->active) + die("BUG: attempt to commit unlocked object"); + + if (lk->filename.len <= LOCK_SUFFIX_LEN || + strcmp(lk->filename.buf + lk->filename.len - LOCK_SUFFIX_LEN, LOCK_SUFFIX)) + die("BUG: lockfile filename corrupt"); + + /* remove ".lock": */ + strbuf_add(&result_file, lk->filename.buf, + lk->filename.len - LOCK_SUFFIX_LEN); + err = commit_lock_file_to(lk, result_file.buf); + strbuf_reset(&result_file); + return err; +} + int hold_locked_index(struct lock_file *lk, int die_on_error) { return hold_lock_file_for_update(lk, get_index_file(), diff --git a/read-cache.c b/read-cache.c index 91bf876ee6..e887e23fb6 100644 --- a/read-cache.c +++ b/read-cache.c @@ -2041,17 +2041,10 @@ void set_alternate_index_output(const char *name) static int commit_locked_index(struct lock_file *lk) { - if (alternate_index_output) { - if (close_lock_file(lk)) - return -1; - if (rename(lk->filename.buf, alternate_index_output)) - return -1; - lk->active = 0; - strbuf_reset(&lk->filename); - return 0; - } else { + if (alternate_index_output) + return commit_lock_file_to(lk, alternate_index_output); + else return commit_lock_file(lk); - } } static int do_write_locked_index(struct index_state *istate, struct lock_file *lock, -- cgit v1.3-5-g9baa From 47ba4662bfd755829edd24428cf2e4bc492d70a6 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 1 Oct 2014 12:28:37 +0200 Subject: lockfile: rename LOCK_NODEREF to LOCK_NO_DEREF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes it harder to misread the name as LOCK_NODE_REF. Suggested-by: Torsten Bögershausen Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- Documentation/technical/api-lockfile.txt | 4 ++-- cache.h | 2 +- lockfile.c | 2 +- refs.c | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) (limited to 'cache.h') diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt index aa7d822900..a3cb69b968 100644 --- a/Documentation/technical/api-lockfile.txt +++ b/Documentation/technical/api-lockfile.txt @@ -110,11 +110,11 @@ Flags The following flags can be passed to `hold_lock_file_for_update` or `hold_lock_file_for_append`: -LOCK_NODEREF:: +LOCK_NO_DEREF:: Usually symbolic links in the destination path are resolved and the lockfile is created by adding ".lock" to the resolved - path. If `LOCK_NODEREF` is set, then the lockfile is created + path. If `LOCK_NO_DEREF` is set, then the lockfile is created by adding ".lock" to the path argument itself. This option is used, for example, when locking a symbolic reference, which for backwards-compatibility reasons can be a symbolic link diff --git a/cache.h b/cache.h index 414e93ca6b..7ea4e81257 100644 --- a/cache.h +++ b/cache.h @@ -583,7 +583,7 @@ struct lock_file { struct strbuf filename; }; #define LOCK_DIE_ON_ERROR 1 -#define LOCK_NODEREF 2 +#define LOCK_NO_DEREF 2 extern int unable_to_lock_error(const char *path, int err); extern void unable_to_lock_message(const char *path, int err, struct strbuf *buf); diff --git a/lockfile.c b/lockfile.c index cf7f4d0470..a1cc08a5ff 100644 --- a/lockfile.c +++ b/lockfile.c @@ -169,7 +169,7 @@ static int lock_file(struct lock_file *lk, const char *path, int flags) } strbuf_add(&lk->filename, path, pathlen); - if (!(flags & LOCK_NODEREF)) + if (!(flags & LOCK_NO_DEREF)) resolve_symlink(&lk->filename); strbuf_addstr(&lk->filename, LOCK_SUFFIX); lk->fd = open(lk->filename.buf, O_RDWR | O_CREAT | O_EXCL, 0666); diff --git a/refs.c b/refs.c index 598f4eb9e7..c10eaff57f 100644 --- a/refs.c +++ b/refs.c @@ -2192,7 +2192,7 @@ static struct ref_lock *lock_ref_sha1_basic(const char *refname, lflags = 0; if (flags & REF_NODEREF) { refname = orig_refname; - lflags |= LOCK_NODEREF; + lflags |= LOCK_NO_DEREF; } lock->ref_name = xstrdup(refname); lock->orig_ref_name = xstrdup(orig_refname); -- cgit v1.3-5-g9baa From ec38b4e482e96e62762452cab5714e55abdb48c3 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 1 Oct 2014 12:28:39 +0200 Subject: get_locked_file_path(): new function Add a function to return the path of the file that is locked by a lock_file object. This reduces the knowledge that callers have to have about the lock_file layout. Suggested-by: Ronnie Sahlberg Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- Documentation/technical/api-lockfile.txt | 5 +++++ cache.h | 1 + lockfile.c | 9 +++++++++ refs.c | 4 +--- 4 files changed, 16 insertions(+), 3 deletions(-) (limited to 'cache.h') diff --git a/Documentation/technical/api-lockfile.txt b/Documentation/technical/api-lockfile.txt index a3cb69b968..d4484d154d 100644 --- a/Documentation/technical/api-lockfile.txt +++ b/Documentation/technical/api-lockfile.txt @@ -143,6 +143,11 @@ hold_lock_file_for_append:: the existing contents of the file (if any) to the lockfile and position its write pointer at the end of the file. +get_locked_file_path:: + + Return the path of the file that is locked by the specified + lock_file object. The caller must free the memory. + commit_lock_file:: Take a pointer to the `struct lock_file` initialized with an diff --git a/cache.h b/cache.h index 7ea4e81257..d19e57ff64 100644 --- a/cache.h +++ b/cache.h @@ -590,6 +590,7 @@ extern void unable_to_lock_message(const char *path, int err, extern NORETURN void unable_to_lock_die(const char *path, int err); extern int hold_lock_file_for_update(struct lock_file *, const char *path, int); extern int hold_lock_file_for_append(struct lock_file *, const char *path, int); +extern char *get_locked_file_path(struct lock_file *); extern int commit_lock_file_to(struct lock_file *, const char *path); extern int commit_lock_file(struct lock_file *); extern int reopen_lock_file(struct lock_file *); diff --git a/lockfile.c b/lockfile.c index 0a8c3c881e..c51c6ec69d 100644 --- a/lockfile.c +++ b/lockfile.c @@ -257,6 +257,15 @@ int hold_lock_file_for_append(struct lock_file *lk, const char *path, int flags) return fd; } +char *get_locked_file_path(struct lock_file *lk) +{ + if (!lk->active) + die("BUG: get_locked_file_path() called for unlocked object"); + if (lk->filename.len <= LOCK_SUFFIX_LEN) + die("BUG: get_locked_file_path() called for malformed lock object"); + return xmemdupz(lk->filename.buf, lk->filename.len - LOCK_SUFFIX_LEN); +} + int close_lock_file(struct lock_file *lk) { int fd = lk->fd; diff --git a/refs.c b/refs.c index c10eaff57f..e40c47edce 100644 --- a/refs.c +++ b/refs.c @@ -2606,9 +2606,7 @@ static int delete_ref_loose(struct ref_lock *lock, int flag) * loose. The loose file name is the same as the * lockfile name, minus ".lock": */ - char *loose_filename = xmemdupz( - lock->lk->filename.buf, - lock->lk->filename.len - LOCK_SUFFIX_LEN); + char *loose_filename = get_locked_file_path(lock->lk); int err = unlink_or_warn(loose_filename); free(loose_filename); if (err && errno != ENOENT) -- cgit v1.3-5-g9baa From 697cc8efd944a32ca472337cd6640004c474b788 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Wed, 1 Oct 2014 12:28:42 +0200 Subject: lockfile.h: extract new header file for the functions in lockfile.c Move the interface declaration for the functions in lockfile.c from cache.h to a new file, lockfile.h. Add #includes where necessary (and remove some redundant includes of cache.h by files that already include builtin.h). Move the documentation of the lock_file state diagram from lockfile.c to the new header file. Signed-off-by: Michael Haggerty Signed-off-by: Junio C Hamano --- builtin/add.c | 1 + builtin/apply.c | 1 + builtin/checkout-index.c | 2 +- builtin/checkout.c | 2 +- builtin/clone.c | 1 + builtin/commit.c | 1 + builtin/describe.c | 1 + builtin/diff.c | 1 + builtin/gc.c | 2 +- builtin/merge.c | 1 + builtin/mv.c | 2 +- builtin/read-tree.c | 1 + builtin/receive-pack.c | 1 + builtin/reflog.c | 2 +- builtin/reset.c | 1 + builtin/rm.c | 2 +- builtin/update-index.c | 1 + bundle.c | 1 + cache-tree.c | 1 + cache.h | 27 +--------------- config.c | 1 + credential-store.c | 1 + fast-import.c | 1 + fetch-pack.c | 1 + lockfile.c | 52 +----------------------------- lockfile.h | 84 ++++++++++++++++++++++++++++++++++++++++++++++++ merge-recursive.c | 1 + merge.c | 1 + read-cache.c | 1 + refs.c | 1 + rerere.c | 1 + sequencer.c | 1 + sha1_file.c | 1 + shallow.c | 1 + test-scrap-cache-tree.c | 1 + 35 files changed, 118 insertions(+), 83 deletions(-) create mode 100644 lockfile.h (limited to 'cache.h') diff --git a/builtin/add.c b/builtin/add.c index 352b85e8db..ae6d3e262b 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -5,6 +5,7 @@ */ #include "cache.h" #include "builtin.h" +#include "lockfile.h" #include "dir.h" #include "pathspec.h" #include "exec_cmd.h" diff --git a/builtin/apply.c b/builtin/apply.c index 8714a88720..69efb0e4df 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -7,6 +7,7 @@ * */ #include "cache.h" +#include "lockfile.h" #include "cache-tree.h" #include "quote.h" #include "blob.h" diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c index 05edd9e1df..383dccf93e 100644 --- a/builtin/checkout-index.c +++ b/builtin/checkout-index.c @@ -5,7 +5,7 @@ * */ #include "builtin.h" -#include "cache.h" +#include "lockfile.h" #include "quote.h" #include "cache-tree.h" #include "parse-options.h" diff --git a/builtin/checkout.c b/builtin/checkout.c index 8afdf2b5c4..570bb09c4f 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1,5 +1,5 @@ -#include "cache.h" #include "builtin.h" +#include "lockfile.h" #include "parse-options.h" #include "refs.h" #include "commit.h" diff --git a/builtin/clone.c b/builtin/clone.c index 3927edfb6e..d3bf9532d6 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -9,6 +9,7 @@ */ #include "builtin.h" +#include "lockfile.h" #include "parse-options.h" #include "fetch-pack.h" #include "refs.h" diff --git a/builtin/commit.c b/builtin/commit.c index f55e80986b..c2300185ec 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -6,6 +6,7 @@ */ #include "cache.h" +#include "lockfile.h" #include "cache-tree.h" #include "color.h" #include "dir.h" diff --git a/builtin/describe.c b/builtin/describe.c index ee6a3b998f..9103193b4f 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "lockfile.h" #include "commit.h" #include "tag.h" #include "refs.h" diff --git a/builtin/diff.c b/builtin/diff.c index 0f247d2400..4326fa56bf 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -4,6 +4,7 @@ * Copyright (c) 2006 Junio C Hamano */ #include "cache.h" +#include "lockfile.h" #include "color.h" #include "commit.h" #include "blob.h" diff --git a/builtin/gc.c b/builtin/gc.c index ced1456e1e..005adbebea 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -11,7 +11,7 @@ */ #include "builtin.h" -#include "cache.h" +#include "lockfile.h" #include "parse-options.h" #include "run-command.h" #include "sigchain.h" diff --git a/builtin/merge.c b/builtin/merge.c index be07f27487..4513fadc5f 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -9,6 +9,7 @@ #include "cache.h" #include "parse-options.h" #include "builtin.h" +#include "lockfile.h" #include "run-command.h" #include "diff.h" #include "refs.h" diff --git a/builtin/mv.c b/builtin/mv.c index 8883baa903..563d05ba1a 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -3,8 +3,8 @@ * * Copyright (C) 2006 Johannes Schindelin */ -#include "cache.h" #include "builtin.h" +#include "lockfile.h" #include "dir.h" #include "cache-tree.h" #include "string-list.h" diff --git a/builtin/read-tree.c b/builtin/read-tree.c index e7e1c33a7f..43b47f72f1 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -5,6 +5,7 @@ */ #include "cache.h" +#include "lockfile.h" #include "object.h" #include "tree.h" #include "tree-walk.h" diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index daf0600ca3..10fa25d7b5 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1,4 +1,5 @@ #include "builtin.h" +#include "lockfile.h" #include "pack.h" #include "refs.h" #include "pkt-line.h" diff --git a/builtin/reflog.c b/builtin/reflog.c index 7c78b15c14..b6388f75b0 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -1,5 +1,5 @@ -#include "cache.h" #include "builtin.h" +#include "lockfile.h" #include "commit.h" #include "refs.h" #include "dir.h" diff --git a/builtin/reset.c b/builtin/reset.c index 855d478e3b..4c08ddc1ca 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -8,6 +8,7 @@ * Copyright (c) 2005, 2006 Linus Torvalds and Junio C Hamano */ #include "builtin.h" +#include "lockfile.h" #include "tag.h" #include "object.h" #include "commit.h" diff --git a/builtin/rm.c b/builtin/rm.c index 2b61d3bd41..d8a9c86dd1 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -3,8 +3,8 @@ * * Copyright (C) Linus Torvalds 2006 */ -#include "cache.h" #include "builtin.h" +#include "lockfile.h" #include "dir.h" #include "cache-tree.h" #include "tree-walk.h" diff --git a/builtin/update-index.c b/builtin/update-index.c index 6c9598891d..b0e3dc9105 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -4,6 +4,7 @@ * Copyright (C) Linus Torvalds, 2005 */ #include "cache.h" +#include "lockfile.h" #include "quote.h" #include "cache-tree.h" #include "tree-walk.h" diff --git a/bundle.c b/bundle.c index b2b89fe862..891a3cacda 100644 --- a/bundle.c +++ b/bundle.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "lockfile.h" #include "bundle.h" #include "object.h" #include "commit.h" diff --git a/cache-tree.c b/cache-tree.c index 75a54fdc72..215202c42d 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "lockfile.h" #include "tree.h" #include "tree-walk.h" #include "cache-tree.h" diff --git a/cache.h b/cache.h index d19e57ff64..b71ceb2d8d 100644 --- a/cache.h +++ b/cache.h @@ -570,36 +570,11 @@ extern void fill_stat_cache_info(struct cache_entry *ce, struct stat *st); #define REFRESH_IN_PORCELAIN 0x0020 /* user friendly output, not "needs update" */ extern int refresh_index(struct index_state *, unsigned int flags, const struct pathspec *pathspec, char *seen, const char *header_msg); -/* String appended to a filename to derive the lockfile name: */ -#define LOCK_SUFFIX ".lock" -#define LOCK_SUFFIX_LEN 5 - -struct lock_file { - struct lock_file *volatile next; - volatile sig_atomic_t active; - volatile int fd; - volatile pid_t owner; - char on_list; - struct strbuf filename; -}; -#define LOCK_DIE_ON_ERROR 1 -#define LOCK_NO_DEREF 2 -extern int unable_to_lock_error(const char *path, int err); -extern void unable_to_lock_message(const char *path, int err, - struct strbuf *buf); -extern NORETURN void unable_to_lock_die(const char *path, int err); -extern int hold_lock_file_for_update(struct lock_file *, const char *path, int); -extern int hold_lock_file_for_append(struct lock_file *, const char *path, int); -extern char *get_locked_file_path(struct lock_file *); -extern int commit_lock_file_to(struct lock_file *, const char *path); -extern int commit_lock_file(struct lock_file *); -extern int reopen_lock_file(struct lock_file *); extern void update_index_if_able(struct index_state *, struct lock_file *); extern int hold_locked_index(struct lock_file *, int); extern void set_alternate_index_output(const char *); -extern int close_lock_file(struct lock_file *); -extern void rollback_lock_file(struct lock_file *); + extern int delete_ref(const char *, const unsigned char *sha1, int delopt); /* Environment bits from configuration mechanism */ diff --git a/config.c b/config.c index 21107797d3..c31d4d2486 100644 --- a/config.c +++ b/config.c @@ -6,6 +6,7 @@ * */ #include "cache.h" +#include "lockfile.h" #include "exec_cmd.h" #include "strbuf.h" #include "quote.h" diff --git a/credential-store.c b/credential-store.c index f9146e576f..d435514cbe 100644 --- a/credential-store.c +++ b/credential-store.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "lockfile.h" #include "credential.h" #include "string-list.h" #include "parse-options.h" diff --git a/fast-import.c b/fast-import.c index 783c6840b5..deadc33f94 100644 --- a/fast-import.c +++ b/fast-import.c @@ -153,6 +153,7 @@ Format of STDIN stream: #include "builtin.h" #include "cache.h" +#include "lockfile.h" #include "object.h" #include "blob.h" #include "tree.h" diff --git a/fetch-pack.c b/fetch-pack.c index 7487aa7306..655ee64256 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "lockfile.h" #include "refs.h" #include "pkt-line.h" #include "commit.h" diff --git a/lockfile.c b/lockfile.c index 63f4e94bce..d27e61cafc 100644 --- a/lockfile.c +++ b/lockfile.c @@ -2,59 +2,9 @@ * Copyright (c) 2005, Junio C Hamano */ #include "cache.h" +#include "lockfile.h" #include "sigchain.h" -/* - * File write-locks as used by Git. - * - * For an overview of how to use the lockfile API, please see - * - * Documentation/technical/api-lockfile.txt - * - * This module keeps track of all locked files in lock_file_list for - * use at cleanup. This list and the lock_file objects that comprise - * it must be kept in self-consistent states at all time, because the - * program can be interrupted any time by a signal, in which case the - * signal handler will walk through the list attempting to clean up - * any open lock files. - * - * A lockfile is owned by the process that created it. The lock_file - * object has an "owner" field that records its owner. This field is - * used to prevent a forked process from closing a lockfile created by - * its parent. - * - * The possible states of a lock_file object are as follows: - * - * - Uninitialized. In this state the object's on_list field must be - * zero but the rest of its contents need not be initialized. As - * soon as the object is used in any way, it is irrevocably - * registered in the lock_file_list, and on_list is set. - * - * - Locked, lockfile open (after hold_lock_file_for_update(), - * hold_lock_file_for_append(), or reopen_lock_file()). In this - * state: - * - the lockfile exists - * - active is set - * - filename holds the filename of the lockfile - * - fd holds a file descriptor open for writing to the lockfile - * - owner holds the PID of the process that locked the file - * - * - Locked, lockfile closed (after successful close_lock_file()). - * Same as the previous state, except that the lockfile is closed - * and fd is -1. - * - * - Unlocked (after commit_lock_file(), commit_lock_file_to(), - * rollback_lock_file(), a failed attempt to lock, or a failed - * close_lock_file()). In this state: - * - active is unset - * - filename is empty (usually, though there are transitory - * states in which this condition doesn't hold). Client code should - * *not* rely on the filename being empty in this state. - * - fd is -1 - * - the object is left registered in the lock_file_list, and - * on_list is set. - */ - static struct lock_file *volatile lock_file_list; static void remove_lock_files(void) diff --git a/lockfile.h b/lockfile.h new file mode 100644 index 0000000000..9059e8958f --- /dev/null +++ b/lockfile.h @@ -0,0 +1,84 @@ +#ifndef LOCKFILE_H +#define LOCKFILE_H + +/* + * File write-locks as used by Git. + * + * For an overview of how to use the lockfile API, please see + * + * Documentation/technical/api-lockfile.txt + * + * This module keeps track of all locked files in lock_file_list for + * use at cleanup. This list and the lock_file objects that comprise + * it must be kept in self-consistent states at all time, because the + * program can be interrupted any time by a signal, in which case the + * signal handler will walk through the list attempting to clean up + * any open lock files. + * + * A lockfile is owned by the process that created it. The lock_file + * object has an "owner" field that records its owner. This field is + * used to prevent a forked process from closing a lockfile created by + * its parent. + * + * The possible states of a lock_file object are as follows: + * + * - Uninitialized. In this state the object's on_list field must be + * zero but the rest of its contents need not be initialized. As + * soon as the object is used in any way, it is irrevocably + * registered in the lock_file_list, and on_list is set. + * + * - Locked, lockfile open (after hold_lock_file_for_update(), + * hold_lock_file_for_append(), or reopen_lock_file()). In this + * state: + * - the lockfile exists + * - active is set + * - filename holds the filename of the lockfile + * - fd holds a file descriptor open for writing to the lockfile + * - owner holds the PID of the process that locked the file + * + * - Locked, lockfile closed (after successful close_lock_file()). + * Same as the previous state, except that the lockfile is closed + * and fd is -1. + * + * - Unlocked (after commit_lock_file(), commit_lock_file_to(), + * rollback_lock_file(), a failed attempt to lock, or a failed + * close_lock_file()). In this state: + * - active is unset + * - filename is empty (usually, though there are transitory + * states in which this condition doesn't hold). Client code should + * *not* rely on the filename being empty in this state. + * - fd is -1 + * - the object is left registered in the lock_file_list, and + * on_list is set. + */ + +struct lock_file { + struct lock_file *volatile next; + volatile sig_atomic_t active; + volatile int fd; + volatile pid_t owner; + char on_list; + struct strbuf filename; +}; + +/* String appended to a filename to derive the lockfile name: */ +#define LOCK_SUFFIX ".lock" +#define LOCK_SUFFIX_LEN 5 + +#define LOCK_DIE_ON_ERROR 1 +#define LOCK_NO_DEREF 2 + +extern int unable_to_lock_error(const char *path, int err); +extern void unable_to_lock_message(const char *path, int err, + struct strbuf *buf); +extern NORETURN void unable_to_lock_die(const char *path, int err); +extern int hold_lock_file_for_update(struct lock_file *, const char *path, int); +extern int hold_lock_file_for_append(struct lock_file *, const char *path, int); +extern char *get_locked_file_path(struct lock_file *); +extern int commit_lock_file_to(struct lock_file *, const char *path); +extern int commit_lock_file(struct lock_file *); +extern int reopen_lock_file(struct lock_file *); +extern int close_lock_file(struct lock_file *); +extern void rollback_lock_file(struct lock_file *); + +#endif /* LOCKFILE_H */ diff --git a/merge-recursive.c b/merge-recursive.c index 8ad4be897d..4b0884b7bb 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -5,6 +5,7 @@ */ #include "advice.h" #include "cache.h" +#include "lockfile.h" #include "cache-tree.h" #include "commit.h" #include "blob.h" diff --git a/merge.c b/merge.c index 74ced7f70b..fcff632bd6 100644 --- a/merge.c +++ b/merge.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "lockfile.h" #include "commit.h" #include "run-command.h" #include "resolve-undo.h" diff --git a/read-cache.c b/read-cache.c index 9f137e7f27..8f3e9eb314 100644 --- a/read-cache.c +++ b/read-cache.c @@ -5,6 +5,7 @@ */ #define NO_THE_INDEX_COMPATIBILITY_MACROS #include "cache.h" +#include "lockfile.h" #include "cache-tree.h" #include "refs.h" #include "dir.h" diff --git a/refs.c b/refs.c index e40c47edce..1d73f1daf5 100644 --- a/refs.c +++ b/refs.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "lockfile.h" #include "refs.h" #include "object.h" #include "tag.h" diff --git a/rerere.c b/rerere.c index 20b18add42..1b0555f1a5 100644 --- a/rerere.c +++ b/rerere.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "lockfile.h" #include "string-list.h" #include "rerere.h" #include "xdiff-interface.h" diff --git a/sequencer.c b/sequencer.c index 5e8a207474..1b9a35e587 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "lockfile.h" #include "sequencer.h" #include "dir.h" #include "object.h" diff --git a/sha1_file.c b/sha1_file.c index c08c0cbea8..46ff273e27 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -8,6 +8,7 @@ */ #include "cache.h" #include "string-list.h" +#include "lockfile.h" #include "delta.h" #include "pack.h" #include "blob.h" diff --git a/shallow.c b/shallow.c index 4919baf772..bd7569e815 100644 --- a/shallow.c +++ b/shallow.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "lockfile.h" #include "commit.h" #include "tag.h" #include "pkt-line.h" diff --git a/test-scrap-cache-tree.c b/test-scrap-cache-tree.c index 9ebcbca9d2..6efee31a48 100644 --- a/test-scrap-cache-tree.c +++ b/test-scrap-cache-tree.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "lockfile.h" #include "tree.h" #include "cache-tree.h" -- cgit v1.3-5-g9baa