Re: [PATCH 1/2] Check order when reading index

From: Jaime Soriano Pastor <jsorianopastor@gmail.com>
Date: Aug 27, 2014, 22:13 UTC
Message-ID: <CAPuZ2NHTz2Qu3oorRVTHSb9E-gC_d1kNniVmtUPtpAqPVrUDtA@mail.gmail.com>
In-Reply-To: <xmqq61hdd4e9.fsf@gitster.dls.corp.google.com>

On Wed, Aug 27, 2014 at 11:11 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 6 quoted lines

> I was asking for an answer more from what you know about the code.
> For example, would read_index_unmerged() choke if the index has two
> or more stage #1 (or stage #3) entries for the same path (provided
> that the index is otherwise normal, i.e. no stage #0 entry for that
> path, entries are sorted by pathname order and stages are in an
> order that does not decrease)?

Oh, ok :) Re-reading the code a bit I think that there can be a potential problem in the add_index_entry_with_check() function. It's currently implemented to allow an only entry for each stage and each path, if an entry for a path and a stage is being added, and another one existed before, the old one is replaced, but just the first one, so adding an entry to stage #1 in an index with multiple entries at stage #1 would replace the first occurence, but not the rest, what could not be expected. The user could maybe expect that all entries are replaced, or only an specific one. If an stage #0 entry is added and there are multiple entries for any of the higher-stage entries there wouldn't be any problem as this function removes all the higher-stage entries for the same path without checking the stage. This last case is the one in read_index_unmerged().