lock_file: make function-local locks non-static

Martin Ågren · gitster · commit b227586831ed · 2018-05-10T14:54:45.000+09:00
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 076aa2c (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> Signed-off-by: Junio C Hamano <gitster@pobox.com>
diff --git 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
@@ -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
@@ -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
@@ -233,7 +233,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
@@ -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
@@ -875,7 +875,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
@@ -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
@@ -1858,7 +1858,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
@@ -2989,7 +2989,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
@@ -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;
 

Original file line number	Diff line number	Diff line change
`@@ -4058,7 +4058,7 @@ static int build_fake_ancestor(struct apply_state state, struct patch list)`
`4058`	`4058`	`{`
`4059`	`4059`	`struct patch *patch;`
`4060`	`4060`	`struct index_state result = { NULL };`
`4061`		`- static struct lock_file lock;`
	`4061`	`+ struct lock_file lock = LOCK_INIT;`
`4062`	`4062`	`int res;`
`4063`	`4063`
`4064`	`4064`	`/* Once we start supporting the reverse patch, it may be`
Original file line number	Diff line number	Diff line change
`@@ -612,7 +612,7 @@ int cmd_describe(int argc, const char *argv, const char prefix)`
`612`	`612`	`suffix = broken;`
`613`	`613`	`}`
`614`	`614`	`} else if (dirty) {`
`615`		`- static struct lock_file index_lock;`
	`615`	`+ struct lock_file index_lock = LOCK_INIT;`
`616`	`616`	`struct rev_info revs;`
`617`	`617`	`struct argv_array args = ARGV_ARRAY_INIT;`
`618`	`618`	`int fd, result;`
Original file line number	Diff line number	Diff line change
`@@ -233,7 +233,7 @@ static int need_to_gc(void)`
`233`	`233`	`/* return NULL on success, else hostname running the gc */`
`234`	`234`	`static const char lock_repo_for_gc(int force, pid_t ret_pid)`
`235`	`235`	`{`
`236`		`- static struct lock_file lock;`
	`236`	`+ struct lock_file lock = LOCK_INIT;`
`237`	`237`	`char my_host[HOST_NAME_MAX + 1];`
`238`	`238`	`struct strbuf sb = STRBUF_INIT;`
`239`	`239`	`struct stat st;`
Original file line number	Diff line number	Diff line change
`@@ -875,7 +875,7 @@ static void refuse_unconfigured_deny_delete_current(void)`
`875`	`875`	`static int command_singleton_iterator(void cb_data, struct object_id oid);`
`876`	`876`	`static int update_shallow_ref(struct command cmd, struct shallow_info si)`
`877`	`877`	`{`
`878`		`- static struct lock_file shallow_lock;`
	`878`	`+ struct lock_file shallow_lock = LOCK_INIT;`
`879`	`879`	`struct oid_array extra = OID_ARRAY_INIT;`
`880`	`880`	`struct check_connected_options opt = CHECK_CONNECTED_INIT;`
`881`	`881`	`uint32_t mask = 1 << (cmd->index % 32);`
Original file line number	Diff line number	Diff line change
`@@ -409,7 +409,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)`
`409`	`409`	`int create_bundle(struct bundle_header header, const char path,`
`410`	`410`	`int argc, const char **argv)`
`411`	`411`	`{`
`412`		`- static struct lock_file lock;`
	`412`	`+ struct lock_file lock = LOCK_INIT;`
`413`	`413`	`int bundle_fd = -1;`
`414`	`414`	`int bundle_to_stdout;`
`415`	`415`	`int ref_count = 0;`
Original file line number	Diff line number	Diff line change
`@@ -1858,7 +1858,7 @@ static void dump_marks_helper(FILE *f,`
`1858`	`1858`
`1859`	`1859`	`static void dump_marks(void)`
`1860`	`1860`	`{`
`1861`		`- static struct lock_file mark_lock;`
	`1861`	`+ struct lock_file mark_lock = LOCK_INIT;`
`1862`	`1862`	`FILE *f;`
`1863`	`1863`
`1864`	`1864`	`if (!export_marks_file \|\| (import_marks_file && !import_marks_file_done))`
Original file line number	Diff line number	Diff line change
`@@ -2989,7 +2989,7 @@ static int files_reflog_expire(struct ref_store *ref_store,`
`2989`	`2989`	`{`
`2990`	`2990`	`struct files_ref_store *refs =`
`2991`	`2991`	`files_downcast(ref_store, REF_STORE_WRITE, "reflog_expire");`
`2992`		`- static struct lock_file reflog_lock;`
	`2992`	`+ struct lock_file reflog_lock = LOCK_INIT;`
`2993`	`2993`	`struct expire_reflog_cb cb;`
`2994`	`2994`	`struct ref_lock *lock;`
`2995`	`2995`	`struct strbuf log_file_sb = STRBUF_INIT;`
Original file line number	Diff line number	Diff line change
`@@ -353,7 +353,7 @@ void advertise_shallow_grafts(int fd)`
`353`	`353`	`*/`
`354`	`354`	`void prune_shallow(int show_only)`
`355`	`355`	`{`
`356`		`- static struct lock_file shallow_lock;`
	`356`	`+ struct lock_file shallow_lock = LOCK_INIT;`
`357`	`357`	`struct strbuf sb = STRBUF_INIT;`
`358`	`358`	`int fd;`
`359`	`359`