Re: [PATCH v3 01/18] rerere: fix an off-by-one non-bug

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> When ac49f5ca (rerere "remaining", 2011-02-16) split out a new
> helper function check_one_conflict() out of find_conflict()
> function, so that the latter will use the returned value from the
> new helper to update the loop control variable that is an index into
> active_cache[], the new variable incremented the index by one too
> many when it found a path with only stage #1 entry at the very end
> of active_cache[].
>
> This "strange" return value does not have any effect on the loop
> control of two callers of this function, as they all notice that
> active_nr+2 is larger than active_nr just like active_nr+1 is, but
> nevertheless it puzzles the readers when they are trying to figure
> out what the function is trying to do.
>
> In fact, there is no need to do an early return.  The code that
> follows after skipping the stage #1 entry is fully prepared to
> handle a case where the entry is at the very end of active_cache[].
>
> Help future readers from unnecessary confusion by dropping an early
> return.  We skip the stage #1 entry, and if there are stage #2 and
> stage #3 entries for the same path, we diagnose the path as
> THREE_STAGED (otherwise we say PUNTED), and then we skip all entries
> for the same path.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  rerere.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/rerere.c b/rerere.c
> index 31644de..e307711 100644
> --- a/rerere.c
> +++ b/rerere.c
> @@ -369,10 +369,8 @@ static int check_one_conflict(int i, int *type)
>  	}
>  
>  	*type = PUNTED;
> -	if (ce_stage(e) == 1) {
> -		if (active_nr <= ++i)
> -			return i + 1;
> -	}
> +	if (ce_stage(e) == 1)
> +		i++;

As a conflicted index can have multiple stage #1 entries when
dealing with a criss-cross merge, this should probably be

	while (ce_stage(e) == 1)
        	i++;

instead.

>  	/* Only handle regular files with both stages #2 and #3 */
>  	if (i + 1 < active_nr) {