Skip to content

Commit b3e83cc

Browse files
gitstergitster
committed
hold_locked_index(): align error handling with hold_lockfile_for_update()
Callers of the hold_locked_index() function pass 0 when they want to prepare to write a new version of the index file without wishing to die or emit an error message when the request fails (e.g. somebody else already held the lock), and pass 1 when they want the call to die upon failure. This option is called LOCK_DIE_ON_ERROR by the underlying lockfile API, and the hold_locked_index() function translates the paramter to LOCK_DIE_ON_ERROR when calling the hold_lock_file_for_update(). Replace these hardcoded '1' with LOCK_DIE_ON_ERROR and stop translating. Callers other than the ones that are replaced with this change pass '0' to the function; no behaviour change is intended with this patch. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Among the callers of hold_locked_index() that passes 0: - diff.c::refresh_index_quietly() at the end of "git diff" is an opportunistic update; it leaks the lockfile structure but it is just before the program exits and nobody should care. - builtin/describe.c::cmd_describe(), builtin/commit.c::cmd_status(), sequencer.c::read_and_refresh_cache() are all opportunistic updates and they are OK. - builtin/update-index.c::cmd_update_index() takes a lock upfront but we may end up not needing to update the index (i.e. the entries may be fully up-to-date), in which case we do not need to issue an error upon failure to acquire the lock. We do diagnose and die if we indeed need to update, so it is OK. - wt-status.c::require_clean_work_tree() IS BUGGY. It asks silence, does not check the returned value. Compare with callsites like cmd_describe() and cmd_status() to notice that it is wrong to call update_index_if_able() unconditionally.
1 parent 89d38fb commit b3e83cc

File tree

18 files changed

+27
-29
lines changed

18 files changed

+27
-29
lines changed

‎apply.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4688,7 +4688,7 @@ static int apply_patch(struct apply_state *state,
46884688
state->index_file,
46894689
LOCK_DIE_ON_ERROR);
46904690
else
4691-
state->newfd = hold_locked_index(state->lock_file, 1);
4691+
state->newfd = hold_locked_index(state->lock_file, LOCK_DIE_ON_ERROR);
46924692
}
46934693

46944694
if (state->check_index && read_apply_cache(state) < 0) {

‎builtin/add.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
361361
add_new_files = !take_worktree_changes && !refresh_only;
362362
require_pathspec = !(take_worktree_changes || (0 < addremove_explicit));
363363

364-
hold_locked_index(&lock_file, 1);
364+
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
365365

366366
flags = ((verbose ? ADD_CACHE_VERBOSE : 0) |
367367
(show_only ? ADD_CACHE_PRETEND : 0) |

‎builtin/am.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1119,7 +1119,7 @@ static void refresh_and_write_cache(void)
11191119
{
11201120
struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
11211121

1122-
hold_locked_index(lock_file, 1);
1122+
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
11231123
refresh_cache(REFRESH_QUIET);
11241124
if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
11251125
die(_("unable to write index file"));
@@ -1976,7 +1976,7 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
19761976
return -1;
19771977

19781978
lock_file = xcalloc(1, sizeof(struct lock_file));
1979-
hold_locked_index(lock_file, 1);
1979+
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
19801980

19811981
refresh_cache(REFRESH_QUIET);
19821982

@@ -2016,7 +2016,7 @@ static int merge_tree(struct tree *tree)
20162016
return -1;
20172017

20182018
lock_file = xcalloc(1, sizeof(struct lock_file));
2019-
hold_locked_index(lock_file, 1);
2019+
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
20202020

20212021
memset(&opts, 0, sizeof(opts));
20222022
opts.head_idx = 1;

‎builtin/checkout-index.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
205205
if (index_opt && !state.base_dir_len && !to_tempfile) {
206206
state.refresh_cache = 1;
207207
state.istate = &the_index;
208-
newfd = hold_locked_index(&lock_file, 1);
208+
newfd = hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
209209
}
210210

211211
/* Check out named files first */

‎builtin/checkout.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ static int checkout_paths(const struct checkout_opts *opts,
274274

275275
lock_file = xcalloc(1, sizeof(struct lock_file));
276276

277-
hold_locked_index(lock_file, 1);
277+
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
278278
if (read_cache_preload(&opts->pathspec) < 0)
279279
return error(_("index file corrupt"));
280280

@@ -467,7 +467,7 @@ static int merge_working_tree(const struct checkout_opts *opts,
467467
int ret;
468468
struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
469469

470-
hold_locked_index(lock_file, 1);
470+
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
471471
if (read_cache_preload(NULL) < 0)
472472
return error(_("index file corrupt"));
473473

‎builtin/clone.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,7 @@ static int checkout(int submodule_progress)
711711
setup_work_tree();
712712

713713
lock_file = xcalloc(1, sizeof(struct lock_file));
714-
hold_locked_index(lock_file, 1);
714+
hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);
715715

716716
memset(&opts, 0, sizeof opts);
717717
opts.update = 1;

‎builtin/commit.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
351351

352352
if (interactive) {
353353
char *old_index_env = NULL;
354-
hold_locked_index(&index_lock, 1);
354+
hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
355355

356356
refresh_cache_or_die(refresh_flags);
357357

@@ -396,7 +396,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
396396
* (B) on failure, rollback the real index.
397397
*/
398398
if (all || (also && pathspec.nr)) {
399-
hold_locked_index(&index_lock, 1);
399+
hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
400400
add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
401401
refresh_cache_or_die(refresh_flags);
402402
update_main_cache_tree(WRITE_TREE_SILENT);
@@ -416,7 +416,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
416416
* We still need to refresh the index here.
417417
*/
418418
if (!only && !pathspec.nr) {
419-
hold_locked_index(&index_lock, 1);
419+
hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
420420
refresh_cache_or_die(refresh_flags);
421421
if (active_cache_changed
422422
|| !cache_tree_fully_valid(active_cache_tree))
@@ -468,7 +468,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
468468
if (read_cache() < 0)
469469
die(_("cannot read the index"));
470470

471-
hold_locked_index(&index_lock, 1);
471+
hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);
472472
add_remove_files(&partial);
473473
refresh_cache(REFRESH_QUIET);
474474
update_main_cache_tree(WRITE_TREE_SILENT);

‎builtin/merge.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -634,7 +634,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
634634
{
635635
static struct lock_file lock;
636636

637-
hold_locked_index(&lock, 1);
637+
hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
638638
refresh_cache(REFRESH_QUIET);
639639
if (active_cache_changed &&
640640
write_locked_index(&the_index, &lock, COMMIT_LOCK))
@@ -671,7 +671,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
671671
for (j = common; j; j = j->next)
672672
commit_list_insert(j->item, &reversed);
673673

674-
hold_locked_index(&lock, 1);
674+
hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
675675
clean = merge_recursive(&o, head,
676676
remoteheads->item, reversed, &result);
677677
if (clean < 0)
@@ -781,7 +781,7 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
781781
struct commit_list *parents, **pptr = &parents;
782782
static struct lock_file lock;
783783

784-
hold_locked_index(&lock, 1);
784+
hold_locked_index(&lock, LOCK_DIE_ON_ERROR);
785785
refresh_cache(REFRESH_QUIET);
786786
if (active_cache_changed &&
787787
write_locked_index(&the_index, &lock, COMMIT_LOCK))

‎builtin/mv.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
126126
if (--argc < 1)
127127
usage_with_options(builtin_mv_usage, builtin_mv_options);
128128

129-
hold_locked_index(&lock_file, 1);
129+
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
130130
if (read_cache() < 0)
131131
die(_("index file corrupt"));
132132

‎builtin/read-tree.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
150150
argc = parse_options(argc, argv, unused_prefix, read_tree_options,
151151
read_tree_usage, 0);
152152

153-
hold_locked_index(&lock_file, 1);
153+
hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);
154154

155155
prefix_set = opts.prefix ? 1 : 0;
156156
if (1 < opts.merge + opts.reset + prefix_set)

0 commit comments

Comments
 (0)