[PATCH v2 4/5] lock_file: make function-local locks non-static

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
---
 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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux