Re: [BUG] two-way read-tree can write null sha1s into index

From: Jeff King <peff@peff.net>
Date: Jan 7, 2013, 13:46 UTC
Message-ID: <20130107134654.GA22692@sigill.intra.peff.net>
In-Reply-To: <7vzk0pvqvu.fsf@alter.siamese.dyndns.org>

On Thu, Jan 03, 2013 at 02:33:09PM -0800, Junio C Hamano wrote:

Show 12 quoted lines

> Jeff King <peff@peff.net> writes:
> 
> > Oh, I agree it's insane to try to carry through unmerged entries. I'm
> > just concerned that not all code paths are careful enough to check.
> 
> I would actually be surprised if some code path do assume somebody
> might give them an index with conflicting entries in it and guard
> against it.  We have been coding under the "index must exactly match
> the second tree when three-way unpack_trees() begin" requirement
> since day one.  An conflicted entry will appear as "index and HEAD
> not matching" and will cause reject_merge() early in threeway_merge()
> anyway, no?

Hmm. There is code in threeway_merge to handle a few cases:

        /*
         * We start with cases where the index is allowed to match
         * something other than the head: #14(ALT) and #2ALT, where it
         * is permitted to match the result instead.
         */
        /* #14, #14ALT, #2ALT */
        if (remote && !df_conflict_head && head_match && !remote_match) {
                if (index && !same(index, remote) && !same(index, head))
                        return o->gently ? -1 : reject_merge(index, o);
                return merged_entry(remote, index, o);
        }

but I do not think we have to worry, because we know that the index will never match remote, either (and merged_entry has already been taught to be wary of the conflicted bit, anyway). I'm not entirely clear on how that would get triggered if all of the callers avoid operating on a modified index.

-Peff