git/merge.c
Martin Ågren df60cf5789 read-cache: leave lock in right state in write_locked_index()
If the original version of `write_locked_index()` returned with an
error, it didn't roll back the lockfile unless the error occured at the
very end, during closing/committing. See commit 03b866477 (read-cache:
new API write_locked_index instead of write_index/write_cache,
2014-06-13).

In commit 9f41c7a6b (read-cache: close index.lock in do_write_index,
2017-04-26), we learned to close the lock slightly earlier in the
callstack. That was mostly a side-effect of lockfiles being implemented
using temporary files, but didn't cause any real harm.

Recently, commit 076aa2cbd (tempfile: auto-allocate tempfiles on heap,
2017-09-05) introduced a subtle bug. If the temporary file is deleted
(i.e., the lockfile is rolled back), the tempfile-pointer in the `struct
lock_file` will be left dangling. Thus, an attempt to reuse the
lockfile, or even just to roll it back, will induce undefined behavior
-- most likely a crash.

Besides not crashing, we clearly want to make things consistent. The
guarantees which the lockfile-machinery itself provides is A) if we ask
to commit and it fails, roll back, and B) if we ask to close and it
fails, do _not_ roll back. Let's do the same for consistency.

Do not delete the temporary file in `do_write_index()`. One of its
callers, `write_locked_index()` will thereby avoid rolling back the
lock. The other caller, `write_shared_index()`, will delete its
temporary file anyway. Both of these callers will avoid undefined
behavior (crashing).

Teach `write_locked_index(..., COMMIT_LOCK)` to roll back the lock
before returning. If we have already succeeded and committed, it will be
a noop. Simplify the existing callers where we now have a superfluous
call to `rollback_lockfile()`. That should keep future readers from
wondering why the callers are inconsistent.

Signed-off-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-10-07 10:20:56 +09:00

97 lines
2.4 KiB
C

#include "cache.h"
#include "lockfile.h"
#include "commit.h"
#include "run-command.h"
#include "resolve-undo.h"
#include "tree-walk.h"
#include "unpack-trees.h"
#include "dir.h"
static const char *merge_argument(struct commit *commit)
{
if (commit)
return oid_to_hex(&commit->object.oid);
else
return EMPTY_TREE_SHA1_HEX;
}
int try_merge_command(const char *strategy, size_t xopts_nr,
const char **xopts, struct commit_list *common,
const char *head_arg, struct commit_list *remotes)
{
struct argv_array args = ARGV_ARRAY_INIT;
int i, ret;
struct commit_list *j;
argv_array_pushf(&args, "merge-%s", strategy);
for (i = 0; i < xopts_nr; i++)
argv_array_pushf(&args, "--%s", xopts[i]);
for (j = common; j; j = j->next)
argv_array_push(&args, merge_argument(j->item));
argv_array_push(&args, "--");
argv_array_push(&args, head_arg);
for (j = remotes; j; j = j->next)
argv_array_push(&args, merge_argument(j->item));
ret = run_command_v_opt(args.argv, RUN_GIT_CMD);
argv_array_clear(&args);
discard_cache();
if (read_cache() < 0)
die(_("failed to read the cache"));
resolve_undo_clear();
return ret;
}
int checkout_fast_forward(const struct object_id *head,
const struct object_id *remote,
int overwrite_ignore)
{
struct tree *trees[MAX_UNPACK_TREES];
struct unpack_trees_options opts;
struct tree_desc t[MAX_UNPACK_TREES];
int i, nr_trees = 0;
struct dir_struct dir;
struct lock_file lock_file = LOCK_INIT;
refresh_cache(REFRESH_QUIET);
if (hold_locked_index(&lock_file, LOCK_REPORT_ON_ERROR) < 0)
return -1;
memset(&trees, 0, sizeof(trees));
memset(&opts, 0, sizeof(opts));
memset(&t, 0, sizeof(t));
if (overwrite_ignore) {
memset(&dir, 0, sizeof(dir));
dir.flags |= DIR_SHOW_IGNORED;
setup_standard_excludes(&dir);
opts.dir = &dir;
}
opts.head_idx = 1;
opts.src_index = &the_index;
opts.dst_index = &the_index;
opts.update = 1;
opts.verbose_update = 1;
opts.merge = 1;
opts.fn = twoway_merge;
setup_unpack_trees_porcelain(&opts, "merge");
trees[nr_trees] = parse_tree_indirect(head);
if (!trees[nr_trees++])
return -1;
trees[nr_trees] = parse_tree_indirect(remote);
if (!trees[nr_trees++])
return -1;
for (i = 0; i < nr_trees; i++) {
parse_tree(trees[i]);
init_tree_desc(t+i, trees[i]->buffer, trees[i]->size);
}
if (unpack_trees(nr_trees, t, &opts))
return -1;
if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
return error(_("unable to write new index file"));
return 0;
}