[PATCH v2 4/5] lock_file: make function-local locks non-static
- From
- Martin Ågren <martin.agren@gmail.com>
- Date
- May 9, 2018, 20:55 UTC
- Message-ID
- <787d85a2cda820ee5c78400691e7da77b3268fdf.1525898125.git.martin.agren@gmail.com>
- In-Reply-To
- <cover.1525898125.git.martin.agren@gmail.com>
Placing `struct lock_file`s on the stack used to be a bad idea, because the temp- and lockfile-machinery would keep a pointer into the struct. But after 076aa2cbd (tempfile: auto-allocate tempfiles on heap, 2017-09-05), we can safely have lockfiles on the stack. (This applies even if a user returns early, leaving a locked lock behind.)
These `struct lock_file`s are local to their respective functions and we can drop their staticness.
For good measure, I have inspected these sites and come to believe that they always release the lock, with the possible exception of bailing out using `die()` or `exit()` or by returning from a `cmd_foo()`.
As pointed out by Jeff King, it would be bad if someone held on to a `struct lock_file *` for some reason. After some grepping, I agree with his findings: no-one appears to be doing that.
Signed-off-by: Martin Ågren <martin.agren@gmail.com> --- apply.c | 2 +- builtin/describe.c | 2 +- builtin/difftool.c | 2 +- builtin/gc.c | 2 +- builtin/merge.c | 4 ++-- builtin/receive-pack.c | 2 +- bundle.c | 2 +- fast-import.c | 2 +- refs/files-backend.c | 2 +- shallow.c | 2 +- 10 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/apply.c b/apply.c index 7e5792c996..07b14d1127 100644 --- a/apply.c +++ b/apply.c @@ -4058,7 +4058,7 @@ static int build_fake_ancestor(struct apply_state *state, struct patch *list) { struct patch *patch; struct index_state result = { NULL }; - static struct lock_file lock; + struct lock_file lock = LOCK_INIT; int res; /* Once we start supporting the reverse patch, it may be diff --git a/builtin/describe.c b/builtin/describe.c index b5afc45846..8bd95cf371 100644 --- a/builtin/describe.c +++ b/builtin/describe.c @@ -612,7 +612,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix) suffix = broken; } } else if (dirty) { - static struct lock_file index_lock; + struct lock_file index_lock = LOCK_INIT; struct rev_info revs; struct argv_array args = ARGV_ARRAY_INIT; int fd, result; diff --git a/builtin/difftool.c b/builtin/difftool.c index aad0e073ee..162806f238 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -610,7 +610,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix, continue; if (!indices_loaded) { - static struct lock_file lock; + struct lock_file lock = LOCK_INIT; strbuf_reset(&buf); strbuf_addf(&buf, "%s/wtindex", tmpdir); if (hold_lock_file_for_update(&lock, buf.buf, 0) < 0 || diff --git a/builtin/gc.c b/builtin/gc.c index 3e67124eaa..274660d9dc 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -234,7 +234,7 @@ static int need_to_gc(void) /* return NULL on success, else hostname running the gc */ static const char *lock_repo_for_gc(int force, pid_t* ret_pid) { - static struct lock_file lock; + struct lock_file lock = LOCK_INIT; char my_host[HOST_NAME_MAX + 1]; struct strbuf sb = STRBUF_INIT; struct stat st; diff --git a/builtin/merge.c b/builtin/merge.c index 9db5a2cf16..de62b2c5c6 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -647,7 +647,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common, struct commit_list *remoteheads, struct commit *head) { - static struct lock_file lock; + struct lock_file lock = LOCK_INIT; const char *head_arg = "HEAD"; hold_locked_index(&lock, LOCK_DIE_ON_ERROR); @@ -805,7 +805,7 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads) { struct object_id result_tree, result_commit; struct commit_list *parents, **pptr = &parents; - static struct lock_file lock; + struct lock_file lock = LOCK_INIT; hold_locked_index(&lock, LOCK_DIE_ON_ERROR); refresh_cache(REFRESH_QUIET); diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 4b68a28e92..d3cf36cef5 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -876,7 +876,7 @@ static void refuse_unconfigured_deny_delete_current(void) static int command_singleton_iterator(void *cb_data, struct object_id *oid); static int update_shallow_ref(struct command *cmd, struct shallow_info *si) { - static struct lock_file shallow_lock; + struct lock_file shallow_lock = LOCK_INIT; struct oid_array extra = OID_ARRAY_INIT; struct check_connected_options opt = CHECK_CONNECTED_INIT; uint32_t mask = 1 << (cmd->index % 32); diff --git a/bundle.c b/bundle.c index 902c9b5448..160bbfdc64 100644 --- a/bundle.c +++ b/bundle.c @@ -409,7 +409,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs) int create_bundle(struct bundle_header *header, const char *path, int argc, const char **argv) { - static struct lock_file lock; + struct lock_file lock = LOCK_INIT; int bundle_fd = -1; int bundle_to_stdout; int ref_count = 0; diff --git a/fast-import.c b/fast-import.c index 05d1079d23..09443c1a9d 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1817,7 +1817,7 @@ static void dump_marks_helper(FILE *f, static void dump_marks(void) { - static struct lock_file mark_lock; + struct lock_file mark_lock = LOCK_INIT; FILE *f; if (!export_marks_file || (import_marks_file && !import_marks_file_done)) diff --git a/refs/files-backend.c b/refs/files-backend.c index a92a2aa821..7b2da7b8c9 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2995,7 +2995,7 @@ static int files_reflog_expire(struct ref_store *ref_store, { struct files_ref_store *refs = files_downcast(ref_store, REF_STORE_WRITE, "reflog_expire"); - static struct lock_file reflog_lock; + struct lock_file reflog_lock = LOCK_INIT; struct expire_reflog_cb cb; struct ref_lock *lock; struct strbuf log_file_sb = STRBUF_INIT; diff --git a/shallow.c b/shallow.c index df4d44ea7a..85313619ea 100644 --- a/shallow.c +++ b/shallow.c @@ -353,7 +353,7 @@ void advertise_shallow_grafts(int fd) */ void prune_shallow(int show_only) { - static struct lock_file shallow_lock; + struct lock_file shallow_lock = LOCK_INIT; struct strbuf sb = STRBUF_INIT; int fd;
-- 2.17.0.411.g9fd64c8e46