Elijah Newren <newren@xxxxxxxxx> writes: > Make sure we do the index == head check at the beginning of the merge, > and error out immediately if it fails. While we're at it, fix a small > leak in the show-the-error codepath. As the call to repo_index_has_changes() is moved to the very beginning of merge_recursive() and merge_trees(), the workhorse of the merge machinery, merge_trees_internal(), can lose it. > +static int merge_start(struct merge_options *opt, struct tree *head) > +{ > + struct strbuf sb = STRBUF_INIT; > + > + assert(opt->branch1 && opt->branch2); This is a new assertion that did not exist in the original, isn't it? I do not object to new sensible assertions, and I think these two fields must be non-null in a freshly initialized merge_options structure, but shouldn't we be discussing if these two fields should be non-NULL, and if there are other fields in the same structure that we should be adding new assertions on, in a separate step on its own? > + if (repo_index_has_changes(opt->repo, head, &sb)) { > + err(opt, _("Your local changes to the following files would be overwritten by merge:\n %s"), > + sb.buf); > + strbuf_release(&sb); > + return -1; > + } > + > + return 0; > +}