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.

diff --git a/apply.c b/apply.c
index 705cf562f07098aafcd9f6e27105d2105714751f..2ed808d429969ff9516ba1bf166c3fdfa6d63ed5 100644 (file)
--- a/apply.c
+++ b/apply.c
@@ -4688,7 +4688,7 @@ static int apply_patch(struct apply_state *state,
                                                                 state->index_file,
                                                                 LOCK_DIE_ON_ERROR);
                else
                                                                 state->index_file,
                                                                 LOCK_DIE_ON_ERROR);
                else

-                       state->newfd = hold_locked_index(state->lock_file, 1);
+                       state->newfd = hold_locked_index(state->lock_file, LOCK_DIE_ON_ERROR);

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

diff --git a/builtin/add.c b/builtin/add.c
index e8fb80b36e7386fa9a9da91a61893c14ec696be4..9f53f020d0fc7184ac803a4346bdf426a814ed72 100644 (file)
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -361,7 +361,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)
        add_new_files = !take_worktree_changes && !refresh_only;
        require_pathspec = !(take_worktree_changes || (0 < addremove_explicit));
 
        add_new_files = !take_worktree_changes && !refresh_only;
        require_pathspec = !(take_worktree_changes || (0 < addremove_explicit));
 

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

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

diff --git a/builtin/am.c b/builtin/am.c
index 6981f42ce986dc5a9fa76bed8364cee7c6c642aa..bb5da422fc1ae06a8e5ad3735cd0cda69ff5abc1 100644 (file)
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1119,7 +1119,7 @@ static void refresh_and_write_cache(void)
 {
        struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
 
 {
        struct lock_file *lock_file = xcalloc(1, sizeof(struct lock_file));
 

-       hold_locked_index(lock_file, 1);
+       hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);

        refresh_cache(REFRESH_QUIET);
        if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
                die(_("unable to write index file"));
        refresh_cache(REFRESH_QUIET);
        if (write_locked_index(&the_index, lock_file, COMMIT_LOCK))
                die(_("unable to write index file"));


@@ -1976,7 +1976,7 @@ static int fast_forward_to(struct tree *head, struct tree *remote, int reset)
                return -1;
 
        lock_file = xcalloc(1, sizeof(struct lock_file));
                return -1;
 
        lock_file = xcalloc(1, sizeof(struct lock_file));

-       hold_locked_index(lock_file, 1);
+       hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);

 
        refresh_cache(REFRESH_QUIET);
 
 
        refresh_cache(REFRESH_QUIET);
 


@@ -2016,7 +2016,7 @@ static int merge_tree(struct tree *tree)
                return -1;
 
        lock_file = xcalloc(1, sizeof(struct lock_file));
                return -1;
 
        lock_file = xcalloc(1, sizeof(struct lock_file));

-       hold_locked_index(lock_file, 1);
+       hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);

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

diff --git a/builtin/checkout-index.c b/builtin/checkout-index.c
index 30a49d9f424c0a1e04b321b5eb0a5e25462a53af..07631d0c9c59f6ba03f288294797e08cdfe22b7c 100644 (file)
--- a/builtin/checkout-index.c
+++ b/builtin/checkout-index.c
@@ -205,7 +205,7 @@ int cmd_checkout_index(int argc, const char **argv, const char *prefix)
        if (index_opt && !state.base_dir_len && !to_tempfile) {
                state.refresh_cache = 1;
                state.istate = &the_index;
        if (index_opt && !state.base_dir_len && !to_tempfile) {
                state.refresh_cache = 1;
                state.istate = &the_index;

-               newfd = hold_locked_index(&lock_file, 1);
+               newfd = hold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);

        }
 
        /* Check out named files first */
        }
 
        /* Check out named files first */

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 512492aad9099dd2a7aa85391b5c9a8e321c3bfe..bfe685c198cd38b59ffe66fa6b3d3a7bced60824 100644 (file)
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -274,7 +274,7 @@ static int checkout_paths(const struct checkout_opts *opts,
 
        lock_file = xcalloc(1, sizeof(struct lock_file));
 
 
        lock_file = xcalloc(1, sizeof(struct lock_file));
 

-       hold_locked_index(lock_file, 1);
+       hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);

        if (read_cache_preload(&opts->pathspec) < 0)
                return error(_("index file corrupt"));
 
        if (read_cache_preload(&opts->pathspec) < 0)
                return error(_("index file corrupt"));
 


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

-       hold_locked_index(lock_file, 1);
+       hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);

        if (read_cache_preload(NULL) < 0)
                return error(_("index file corrupt"));
 
        if (read_cache_preload(NULL) < 0)
                return error(_("index file corrupt"));
 

diff --git a/builtin/clone.c b/builtin/clone.c
index 6c76a6ed66fef567ca06e3e864b37fc3bed151d5..892bdbfe3f2752a89da69a86efcfa58ab59aee3f 100644 (file)
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -711,7 +711,7 @@ static int checkout(int submodule_progress)
        setup_work_tree();
 
        lock_file = xcalloc(1, sizeof(struct lock_file));
        setup_work_tree();
 
        lock_file = xcalloc(1, sizeof(struct lock_file));

-       hold_locked_index(lock_file, 1);
+       hold_locked_index(lock_file, LOCK_DIE_ON_ERROR);

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

diff --git a/builtin/commit.c b/builtin/commit.c
index 8976c3d29bf817be789f983697f7052fe5769f39..b910e760179a12032433ad20007d47a992a56255 100644 (file)
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -351,7 +351,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
 
        if (interactive) {
                char *old_index_env = NULL;
 
        if (interactive) {
                char *old_index_env = NULL;

-               hold_locked_index(&index_lock, 1);
+               hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);

 
                refresh_cache_or_die(refresh_flags);
 
 
                refresh_cache_or_die(refresh_flags);
 


@@ -396,7 +396,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
         * (B) on failure, rollback the real index.
         */
        if (all || (also && pathspec.nr)) {
         * (B) on failure, rollback the real index.
         */
        if (all || (also && pathspec.nr)) {

-               hold_locked_index(&index_lock, 1);
+               hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);

                add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
                refresh_cache_or_die(refresh_flags);
                update_main_cache_tree(WRITE_TREE_SILENT);
                add_files_to_cache(also ? prefix : NULL, &pathspec, 0);
                refresh_cache_or_die(refresh_flags);
                update_main_cache_tree(WRITE_TREE_SILENT);


@@ -416,7 +416,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
         * We still need to refresh the index here.
         */
        if (!only && !pathspec.nr) {
         * We still need to refresh the index here.
         */
        if (!only && !pathspec.nr) {

-               hold_locked_index(&index_lock, 1);
+               hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);

                refresh_cache_or_die(refresh_flags);
                if (active_cache_changed
                    || !cache_tree_fully_valid(active_cache_tree))
                refresh_cache_or_die(refresh_flags);
                if (active_cache_changed
                    || !cache_tree_fully_valid(active_cache_tree))


@@ -468,7 +468,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
        if (read_cache() < 0)
                die(_("cannot read the index"));
 
        if (read_cache() < 0)
                die(_("cannot read the index"));
 

-       hold_locked_index(&index_lock, 1);
+       hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);

        add_remove_files(&partial);
        refresh_cache(REFRESH_QUIET);
        update_main_cache_tree(WRITE_TREE_SILENT);
        add_remove_files(&partial);
        refresh_cache(REFRESH_QUIET);
        update_main_cache_tree(WRITE_TREE_SILENT);

diff --git a/builtin/merge.c b/builtin/merge.c
index b65eeaa87d303b027230bf2479f78c15a02ca08a..0070bf255612ea9d4826720df7a77dd8ccb762e8 100644 (file)
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -634,7 +634,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
 {
        static struct lock_file lock;
 
 {
        static struct lock_file lock;
 

-       hold_locked_index(&lock, 1);
+       hold_locked_index(&lock, LOCK_DIE_ON_ERROR);

        refresh_cache(REFRESH_QUIET);
        if (active_cache_changed &&
            write_locked_index(&the_index, &lock, COMMIT_LOCK))
        refresh_cache(REFRESH_QUIET);
        if (active_cache_changed &&
            write_locked_index(&the_index, &lock, COMMIT_LOCK))


@@ -671,7 +671,7 @@ static int try_merge_strategy(const char *strategy, struct commit_list *common,
                for (j = common; j; j = j->next)
                        commit_list_insert(j->item, &reversed);
 
                for (j = common; j; j = j->next)
                        commit_list_insert(j->item, &reversed);
 

-               hold_locked_index(&lock, 1);
+               hold_locked_index(&lock, LOCK_DIE_ON_ERROR);

                clean = merge_recursive(&o, head,
                                remoteheads->item, reversed, &result);
                if (clean < 0)
                clean = merge_recursive(&o, head,
                                remoteheads->item, reversed, &result);
                if (clean < 0)


@@ -781,7 +781,7 @@ static int merge_trivial(struct commit *head, struct commit_list *remoteheads)
        struct commit_list *parents, **pptr = &parents;
        static struct lock_file lock;
 
        struct commit_list *parents, **pptr = &parents;
        static struct lock_file lock;
 

-       hold_locked_index(&lock, 1);
+       hold_locked_index(&lock, LOCK_DIE_ON_ERROR);

        refresh_cache(REFRESH_QUIET);
        if (active_cache_changed &&
            write_locked_index(&the_index, &lock, COMMIT_LOCK))
        refresh_cache(REFRESH_QUIET);
        if (active_cache_changed &&
            write_locked_index(&the_index, &lock, COMMIT_LOCK))

diff --git a/builtin/mv.c b/builtin/mv.c
index 2f43877bc9a17c5bef2906a383cc8cdd6f4f5b82..43adf92ba64426a2f312cd6133276a98b20855f1 100644 (file)
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -126,7 +126,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
        if (--argc < 1)
                usage_with_options(builtin_mv_usage, builtin_mv_options);
 
        if (--argc < 1)
                usage_with_options(builtin_mv_usage, builtin_mv_options);
 

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

        if (read_cache() < 0)
                die(_("index file corrupt"));
 
        if (read_cache() < 0)
                die(_("index file corrupt"));
 

diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index 9bd1fd755ef03824442f6c751a9603b95bfe66ee..fa6edb35b21cede4425746d84b41c8bd9e31e1dc 100644 (file)
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -150,7 +150,7 @@ int cmd_read_tree(int argc, const char **argv, const char *unused_prefix)
        argc = parse_options(argc, argv, unused_prefix, read_tree_options,
                             read_tree_usage, 0);
 
        argc = parse_options(argc, argv, unused_prefix, read_tree_options,
                             read_tree_usage, 0);
 

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

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

diff --git a/builtin/reset.c b/builtin/reset.c
index c04ac076dc53b99039768dcdcbf2861e285896c2..8ab915bfcb71ae5d5f4bd1bebe7fd3c8725780b2 100644 (file)
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -354,7 +354,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 
        if (reset_type != SOFT) {
                struct lock_file *lock = xcalloc(1, sizeof(*lock));
 
        if (reset_type != SOFT) {
                struct lock_file *lock = xcalloc(1, sizeof(*lock));

-               hold_locked_index(lock, 1);
+               hold_locked_index(lock, LOCK_DIE_ON_ERROR);

                if (reset_type == MIXED) {
                        int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;
                        if (read_from_tree(&pathspec, &oid, intent_to_add))
                if (reset_type == MIXED) {
                        int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN;
                        if (read_from_tree(&pathspec, &oid, intent_to_add))

diff --git a/builtin/rm.c b/builtin/rm.c
index 3f3e24eb36af03481f4e7b3f4d22d8e4b5904593..7f15a3d7f82a7b610dcb8c6c9f713b485614e2f7 100644 (file)
--- a/builtin/rm.c
+++ b/builtin/rm.c
@@ -292,7 +292,7 @@ int cmd_rm(int argc, const char **argv, const char *prefix)
        if (!index_only)
                setup_work_tree();
 
        if (!index_only)
                setup_work_tree();
 

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

 
        if (read_cache() < 0)
                die(_("index file corrupt"));
 
        if (read_cache() < 0)
                die(_("index file corrupt"));

diff --git a/builtin/update-index.c b/builtin/update-index.c
index f3f07e7f1cb2d952144bf98969b481dc331155bf..d530e89368b42bf1465784b80163b9158a06585a 100644 (file)
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1012,6 +1012,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
        /* We can't free this memory, it becomes part of a linked list parsed atexit() */
        lock_file = xcalloc(1, sizeof(struct lock_file));
 
        /* We can't free this memory, it becomes part of a linked list parsed atexit() */
        lock_file = xcalloc(1, sizeof(struct lock_file));
 

+       /* we will diagnose later if it turns out that we need to update it */

        newfd = hold_locked_index(lock_file, 0);
        if (newfd < 0)
                lock_error = errno;
        newfd = hold_locked_index(lock_file, 0);
        if (newfd < 0)
                lock_error = errno;

diff --git a/merge-recursive.c b/merge-recursive.c
index 9041c2f149c01134ce02119354455894533e713c..8442068716035b61a448514e3fa7957aed51a597 100644 (file)
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -2124,7 +2124,7 @@ int merge_recursive_generic(struct merge_options *o,
                }
        }
 
                }
        }
 

-       hold_locked_index(lock, 1);
+       hold_locked_index(lock, LOCK_DIE_ON_ERROR);

        clean = merge_recursive(o, head_commit, next_commit, ca,
                        result);
        if (clean < 0)
        clean = merge_recursive(o, head_commit, next_commit, ca,
                        result);
        if (clean < 0)

diff --git a/read-cache.c b/read-cache.c
index db5d910642663e73e4e4fc8d91e0caba4445bdb1..f92a912dcb224b3f6da9e9be07a33cfe3b08edcc 100644 (file)
--- a/read-cache.c
+++ b/read-cache.c
@@ -1425,12 +1425,9 @@ static int read_index_extension(struct index_state *istate,
        return 0;
 }
 
        return 0;
 }
 

-int hold_locked_index(struct lock_file *lk, int die_on_error)
+int hold_locked_index(struct lock_file *lk, int lock_flags)

 {
 {

-       return hold_lock_file_for_update(lk, get_index_file(),
-                                        die_on_error
-                                        ? LOCK_DIE_ON_ERROR
-                                        : 0);
+       return hold_lock_file_for_update(lk, get_index_file(), lock_flags);

 }
 
 int read_index(struct index_state *istate)
 }
 
 int read_index(struct index_state *istate)

diff --git a/rerere.c b/rerere.c
index 5d083ca572df0c8a2cb90c0b12c14e15b4134d48..3bd55caf3b0961888bcca06d3c54577cb25f5223 100644 (file)
--- a/rerere.c
+++ b/rerere.c
@@ -708,7 +708,7 @@ static void update_paths(struct string_list *update)
 {
        int i;
 
 {
        int i;
 

-       hold_locked_index(&index_lock, 1);
+       hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);

 
        for (i = 0; i < update->nr; i++) {
                struct string_list_item *item = &update->items[i];
 
        for (i = 0; i < update->nr; i++) {
                struct string_list_item *item = &update->items[i];

diff --git a/sequencer.c b/sequencer.c
index 30b10ba143959bf2618872427f730a9f0e30d369..7fc1e2a5df0ada4816dc3de97e8f6139fbabe635 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -370,7 +370,7 @@ static int do_recursive_merge(struct commit *base, struct commit *next,
        char **xopt;
        static struct lock_file index_lock;
 
        char **xopt;
        static struct lock_file index_lock;
 

-       hold_locked_index(&index_lock, 1);
+       hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);

 
        read_cache();
 
 
        read_cache();
 

diff --git a/t/helper/test-scrap-cache-tree.c b/t/helper/test-scrap-cache-tree.c
index 27fe0405b887671ff0ca4cea1b258aa850e66c8c..d2a63bea4346fb76d38ba43508ee6e60599e41a9 100644 (file)
--- a/t/helper/test-scrap-cache-tree.c
+++ b/t/helper/test-scrap-cache-tree.c
@@ -8,7 +8,7 @@ static struct lock_file index_lock;
 int cmd_main(int ac, const char **av)
 {
        setup_git_directory();
 int cmd_main(int ac, const char **av)
 {
        setup_git_directory();

-       hold_locked_index(&index_lock, 1);
+       hold_locked_index(&index_lock, LOCK_DIE_ON_ERROR);

        if (read_cache() < 0)
                die("unable to read index file");
        active_cache_tree = NULL;
        if (read_cache() < 0)
                die("unable to read index file");
        active_cache_tree = NULL;