Skip to content

Commit 0fa5a2e

Browse files
Martin Ågrengitster
authored andcommitted
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 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.) 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 <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent b227586 commit 0fa5a2e

File tree

6 files changed

+7
-11
lines changed

6 files changed

+7
-11
lines changed

builtin/add.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -265,8 +265,6 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
265265
return 0;
266266
}
267267

268-
static struct lock_file lock_file;
269-
270268
static const char ignore_error[] =
271269
N_("The following paths are ignored by one of your .gitignore files:\n");
272270

@@ -393,6 +391,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
393391
int add_new_files;
394392
int require_pathspec;
395393
char *seen = NULL;
394+
struct lock_file lock_file = LOCK_INIT;
396395

397396
git_config(add_config, NULL);
398397

builtin/mv.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,6 @@ static const char *add_slash(const char *path)
7272
return path;
7373
}
7474

75-
static struct lock_file lock_file;
7675
#define SUBMODULE_WITH_GITDIR ((const char *)1)
7776

7877
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)
131130
enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
132131
struct stat st;
133132
struct string_list src_for_dst = STRING_LIST_INIT_NODUP;
133+
struct lock_file lock_file = LOCK_INIT;
134134

135135
git_config(git_default_config, NULL);
136136

builtin/read-tree.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,15 +107,14 @@ static int git_read_tree_config(const char *var, const char *value, void *cb)
107107
return git_default_config(var, value, cb);
108108
}
109109

110-
static struct lock_file lock_file;
111-
112110
int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
113111
{
114112
int i, stage = 0;
115113
struct object_id oid;
116114
struct tree_desc t[MAX_UNPACK_TREES];
117115
struct unpack_trees_options opts;
118116
int prefix_set = 0;
117+
struct lock_file lock_file = LOCK_INIT;
119118
const struct option read_tree_options[] = {
120119
{ OPTION_CALLBACK, 0, "index-output", NULL, N_("file"),
121120
N_("write resulting index to <file>"),

builtin/rm.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,8 +233,6 @@ static int check_local_mod(struct object_id *head, int index_only)
233233
return errs;
234234
}
235235

236-
static struct lock_file lock_file;
237-
238236
static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0;
239237
static int ignore_unmatch = 0;
240238

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

252250
int cmd_rm(int argc, const char **argv, const char *prefix)
253251
{
252+
struct lock_file lock_file = LOCK_INIT;
254253
int i;
255254
struct pathspec pathspec;
256255
char *seen;

rerere.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -703,10 +703,9 @@ static int merge(const struct rerere_id *id, const char *path)
703703
return ret;
704704
}
705705

706-
static struct lock_file index_lock;
707-
708706
static void update_paths(struct string_list *update)
709707
{
708+
struct lock_file index_lock = LOCK_INIT;
710709
int i;
711710

712711
hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@
33
#include "tree.h"
44
#include "cache-tree.h"
55

6-
static struct lock_file index_lock;
7-
86
int cmd_main(int ac, const char **av)
97
{
8+
struct lock_file index_lock = LOCK_INIT;
9+
1010
setup_git_directory();
1111
hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
1212
if (read_cache() < 0)

0 commit comments

Comments
 (0)