Re: [PATCH] Fix off by one error in prep_exclude.

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 27, 2008, 22:34 UTC
Message-ID: <7v3asiyk2i.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LSU.1.00.0801272043040.23907@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 10 quoted lines

>> but doesn't address the fact that we probably should remove files that 
>> aren't a part of the repository at in the first place.
>
> I am sorry, but I cannot begin to see what this commit tries to 
> accomplish.  Yes, sure, there is an off-by-one error, and your commit 
> message says how that was fixed.  But I miss a description what usage it 
> would affect, i.e. when this bug triggers.
>
> I imagine that you would be as lost as me, reading that commit message 6 
> months from now, trying to understand why that change was made.

Likewise.  The message has somewhat to be desired...

In "struct exclude_stack", prep_exclude() and excluded(), the convention for a path is to express the length of directory part including the trailing slash (e.g. "foo" and "bar/baz" will get baselen=0 and baselen=4 respectively).

The variable current and parameter baselen follow that convention in the codepath the patch touches.

		else {
			cp = strchr(base + current + 1, '/');
			if (!cp)
				die("oops in prep_exclude");
			cp++;
		}
		stk->prev = dir->exclude_stack;
		stk->baselen = cp - base;

is about coming up with the next value for current (which is taken from stk->baselen) to dig one more level.

If base="foo/a/boo" and current=4 (i.e. we are looking at "foo/"), at the point, scanning from (base+current) as Shawn Bohrer's patch suggests means the scan begins at "a/boo" to find the next slash. The existing code skips one letter ('a') and starts scanning from "/boo".

The only case this microoptimization makes difference is when an input is malformed and has double-slash (i.e. path component whose length is zero), like "foo//boo".

Perhaps the "oops part of the issue Johannes found" had a caller that feeds such an incorrect input?