diff options
author | Martin Ågren <martin.agren@gmail.com> | 2018-05-09 22:55:39 +0200 |
---|---|---|
committer | Junio C Hamano <gitster@pobox.com> | 2018-05-10 14:55:40 +0900 |
commit | 0fa5a2ed8d9f6d987f1ea479fe8ea56a26b89303 (patch) | |
tree | 19f1b29e8f3972f281423aa35c9d5711829f1e02 | |
parent | lock_file: make function-local locks non-static (diff) | |
download | tgif-0fa5a2ed8d9f6d987f1ea479fe8ea56a26b89303.tar.xz |
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>
-rw-r--r-- | builtin/add.c | 3 | ||||
-rw-r--r-- | builtin/mv.c | 2 | ||||
-rw-r--r-- | builtin/read-tree.c | 3 | ||||
-rw-r--r-- | builtin/rm.c | 3 | ||||
-rw-r--r-- | rerere.c | 3 | ||||
-rw-r--r-- | t/helper/test-scrap-cache-tree.c | 4 |
6 files changed, 7 insertions, 11 deletions
diff --git a/builtin/add.c b/builtin/add.c index 9ef7fb02d5..80152bcaed 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -265,8 +265,6 @@ static int edit_patch(int argc, const char **argv, const char *prefix) return 0; } -static struct lock_file lock_file; - static const char ignore_error[] = N_("The following paths are ignored by one of your .gitignore files:\n"); @@ -393,6 +391,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) int add_new_files; int require_pathspec; char *seen = NULL; + struct lock_file lock_file = LOCK_INIT; git_config(add_config, NULL); diff --git a/builtin/mv.c b/builtin/mv.c index 6d141f7a53..b4692409e3 100644 --- a/builtin/mv.c +++ b/builtin/mv.c @@ -72,7 +72,6 @@ static const char *add_slash(const char *path) return path; } -static struct lock_file lock_file; #define SUBMODULE_WITH_GITDIR ((const char *)1) 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) enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes; struct stat st; struct string_list src_for_dst = STRING_LIST_INIT_NODUP; + struct lock_file lock_file = LOCK_INIT; git_config(git_default_config, NULL); diff --git a/builtin/read-tree.c b/builtin/read-tree.c index bf87a2710b..ebc43eb805 100644 --- a/builtin/read-tree.c +++ b/builtin/read-tree.c @@ -107,8 +107,6 @@ static int git_read_tree_config(const char *var, const char *value, void *cb) return git_default_config(var, value, cb); } -static struct lock_file lock_file; - int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) { int i, stage = 0; @@ -116,6 +114,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix) struct tree_desc t[MAX_UNPACK_TREES]; struct unpack_trees_options opts; int prefix_set = 0; + struct lock_file lock_file = LOCK_INIT; const struct option read_tree_options[] = { { OPTION_CALLBACK, 0, "index-output", NULL, N_("file"), N_("write resulting index to <file>"), diff --git a/builtin/rm.c b/builtin/rm.c index 4447bb4d0f..0fcb952e9b 100644 --- a/builtin/rm.c +++ b/builtin/rm.c @@ -233,8 +233,6 @@ static int check_local_mod(struct object_id *head, int index_only) return errs; } -static struct lock_file lock_file; - static int show_only = 0, force = 0, index_only = 0, recursive = 0, quiet = 0; static int ignore_unmatch = 0; @@ -251,6 +249,7 @@ static struct option builtin_rm_options[] = { int cmd_rm(int argc, const char **argv, const char *prefix) { + struct lock_file lock_file = LOCK_INIT; int i; struct pathspec pathspec; char *seen; @@ -703,10 +703,9 @@ out: return ret; } -static struct lock_file index_lock; - static void update_paths(struct string_list *update) { + struct lock_file index_lock = LOCK_INIT; int i; hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR); diff --git a/t/helper/test-scrap-cache-tree.c b/t/helper/test-scrap-cache-tree.c index d2a63bea43..34596201d4 100644 --- a/t/helper/test-scrap-cache-tree.c +++ b/t/helper/test-scrap-cache-tree.c @@ -3,10 +3,10 @@ #include "tree.h" #include "cache-tree.h" -static struct lock_file index_lock; - int cmd_main(int ac, const char **av) { + struct lock_file index_lock = LOCK_INIT; + setup_git_directory(); hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR); if (read_cache() < 0) |