@matheusbsbs / matheus-bagliano / commits / b227586831

lock_file: make function-local locks non-static

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> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Martin Ågren committed May 9, 2018 at 22:55 UTC b227586831ed393e1d60629bfedcef01be4b9c22

10 files changed +11 -11

apply.c

+1 -1

@@ -4058,7 +4058,7 @@ static int build_fake_ancestor(struct apply_state *state, struct patch *list)

4058 {

4059 struct patch *patch;

4060 struct index_state result = { NULL };

4061 - static struct lock_file lock;

4061 + struct lock_file lock = LOCK_INIT;

4062 int res;

4063

4064 /* Once we start supporting the reverse patch, it may be

builtin/describe.c

+1 -1

@@ -612,7 +612,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)

612 suffix = broken;

613 }

614 } else if (dirty) {

615 - static struct lock_file index_lock;

615 + struct lock_file index_lock = LOCK_INIT;

616 struct rev_info revs;

617 struct argv_array args = ARGV_ARRAY_INIT;

618 int fd, result;

builtin/difftool.c

+1 -1

@@ -610,7 +610,7 @@ static int run_dir_diff(const char *extcmd, int symlinks, const char *prefix,

610 continue;

611

612 if (!indices_loaded) {

613 - static struct lock_file lock;

613 + struct lock_file lock = LOCK_INIT;

614 strbuf_reset(&buf);

615 strbuf_addf(&buf, "%s/wtindex", tmpdir);

616 if (hold_lock_file_for_update(&lock, buf.buf, 0) < 0 ||

builtin/gc.c

+1 -1

@@ -233,7 +233,7 @@ static int need_to_gc(void)

233 /* return NULL on success, else hostname running the gc */

234 static const char *lock_repo_for_gc(int force, pid_t* ret_pid)

235 {

236 - static struct lock_file lock;

236 + struct lock_file lock = LOCK_INIT;

237 char my_host[HOST_NAME_MAX + 1];

238 struct strbuf sb = STRBUF_INIT;

239 struct stat st;

builtin/merge.c

+2 -2

@@ -647,7 +647,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,

647 struct commit_list *remoteheads,

648 struct commit *head)

649 {

650 - static struct lock_file lock;

650 + struct lock_file lock = LOCK_INIT;

651 const char *head_arg = "HEAD";

652

653 hold_locked_index(&lock, LOCK_DIE_ON_ERROR);

@@ -805,7 +805,7 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)

805 {

806 struct object_id result_tree, result_commit;

807 struct commit_list *parents, **pptr = &parents;

808 - static struct lock_file lock;

808 + struct lock_file lock = LOCK_INIT;

809

810 hold_locked_index(&lock, LOCK_DIE_ON_ERROR);

811 refresh_cache(REFRESH_QUIET);

builtin/receive-pack.c

+1 -1

@@ -875,7 +875,7 @@ static void refuse_unconfigured_deny_delete_current(void)

875 static int command_singleton_iterator(void *cb_data, struct object_id *oid);

876 static int update_shallow_ref(struct command *cmd, struct shallow_info *si)

877 {

878 - static struct lock_file shallow_lock;

878 + struct lock_file shallow_lock = LOCK_INIT;

879 struct oid_array extra = OID_ARRAY_INIT;

880 struct check_connected_options opt = CHECK_CONNECTED_INIT;

881 uint32_t mask = 1 << (cmd->index % 32);

bundle.c

+1 -1

@@ -409,7 +409,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info *revs)

409 int create_bundle(struct bundle_header *header, const char *path,

410 int argc, const char **argv)

411 {

412 - static struct lock_file lock;

412 + struct lock_file lock = LOCK_INIT;

413 int bundle_fd = -1;

414 int bundle_to_stdout;

415 int ref_count = 0;

fast-import.c

+1 -1

@@ -1858,7 +1858,7 @@ static void dump_marks_helper(FILE *f,

1858

1859 static void dump_marks(void)

1860 {

1861 - static struct lock_file mark_lock;

1861 + struct lock_file mark_lock = LOCK_INIT;

1862 FILE *f;

1863

1864 if (!export_marks_file || (import_marks_file && !import_marks_file_done))

refs/files-backend.c

+1 -1

@@ -2989,7 +2989,7 @@ static int files_reflog_expire(struct ref_store *ref_store,

2989 {

2990 struct files_ref_store *refs =

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 struct expire_reflog_cb cb;

2994 struct ref_lock *lock;

2995 struct strbuf log_file_sb = STRBUF_INIT;

shallow.c

+1 -1

@@ -353,7 +353,7 @@ void advertise_shallow_grafts(int fd)

353 */

354 void prune_shallow(int show_only)

355 {

356 - static struct lock_file shallow_lock;

356 + struct lock_file shallow_lock = LOCK_INIT;

357 struct strbuf sb = STRBUF_INIT;

358 int fd;

359