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.ag...@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

Reply via email to