@matheusbsbs / matheus-bagliano / commits / 0fa5a2ed8d

lock_file: move static locks into functions

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.) Each of these `struct lock_file`s is used from within a single function. Move them into the respective functions to make the scope clearer and drop the 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. After this commit, the remaining occurrences of "static struct lock_file" are locks that are used from within different functions. That is, they need to remain static. (Short of more intrusive changes like passing around pointers to non-static locks.) 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 0fa5a2ed8d9f6d987f1ea479fe8ea56a26b89303

6 files changed +7 -11

builtin/add.c

+1 -2

@@ -265,8 +265,6 @@ static int edit_patch(int argc, const char **argv, const char *prefix)

265 return 0;

266 }

267

268 -static struct lock_file lock_file;

269 -

268 static const char ignore_error[] =

269 N_("The following paths are ignored by one of your .gitignore files:\n");

270

@@ -393,6 +391,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)

391 int add_new_files;

392 int require_pathspec;

393 char *seen = NULL;

394 + struct lock_file lock_file = LOCK_INIT;

395

396 git_config(add_config, NULL);

397

builtin/mv.c

+1 -1

@@ -72,7 +72,6 @@ static const char *add_slash(const char *path)

72 return path;

73 }

75 -static struct lock_file lock_file;

75 #define SUBMODULE_WITH_GITDIR ((const char *)1)

77 static void prepare_move_submodule(const char *src, int first,

@@ -131,6 +130,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)

130 enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;

131 struct stat st;

132 struct string_list src_for_dst = STRING_LIST_INIT_NODUP;

133 + struct lock_file lock_file = LOCK_INIT;

134

135 git_config(git_default_config, NULL);

136

builtin/read-tree.c

+1 -2

@@ -107,8 +107,6 @@ static int git_read_tree_config(const char *var, const char *value, void *cb)

107 return git_default_config(var, value, cb);

108 }

109

110 -static struct lock_file lock_file;

111 -

110 int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)

111 {

112 int i, stage = 0;

@@ -116,6 +114,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)

114 struct tree_desc t[MAX_UNPACK_TREES];

115 struct unpack_trees_options opts;

116 int prefix_set = 0;

117 + struct lock_file lock_file = LOCK_INIT;

118 const struct option read_tree_options[] = {

119 { OPTION_CALLBACK, 0, "index-output", NULL, N_("file"),

120 N_("write resulting index to <file>"),

builtin/rm.c

+1 -2

@@ -233,8 +233,6 @@ static int check_local_mod(struct object_id *head, int index_only)

233 return errs;

234 }

235

236 -static struct lock_file lock_file;

237 -

236 static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;

237 static int ignore_unmatch = 0;

238

@@ -251,6 +249,7 @@ static struct option builtin_rm_options[] = {

249

250 int cmd_rm(int argc, const char **argv, const char *prefix)

251 {

252 + struct lock_file lock_file = LOCK_INIT;

253 int i;

254 struct pathspec pathspec;

255 char *seen;

rerere.c

+1 -2

@@ -703,10 +703,9 @@ out:

703 return ret;

704 }

705

706 -static struct lock_file index_lock;

707 -

706 static void update_paths(struct string_list *update)

707 {

708 + struct lock_file index_lock = LOCK_INIT;

709 int i;

710

711 hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);

t/helper/test-scrap-cache-tree.c

+2 -2

@@ -3,10 +3,10 @@

3 #include "tree.h"

4 #include "cache-tree.h"

6 -static struct lock_file index_lock;

7 -

6 int cmd_main(int ac, const char **av)

7 {

8 + struct lock_file index_lock = LOCK_INIT;

9 +

10 setup_git_directory();

11 hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);

12 if (read_cache() < 0)