Re: [PATCH v2] Add support for GIT_CEILING_DIRS

From: David Reiss <dreiss@facebook.com>
Date: May 15, 2008, 16:26 UTC
Message-ID: <482C644F.9090903@facebook.com>
In-Reply-To: <alpine.DEB.1.00.0805151004400.30431@racer>

The problem with this implementation is that it does not distinguish between GIT_CEILING_DIRS being unset and GIT_CEILING_DIRS="/". For example...

cd / sudo git init cd /home git rev-parse --show-prefix

That series of commands works with either version of my patch, but fails with "fatal: Not a git repository" if I apply this change. I am certainly open to changing this code, but I think we will always need two separate values of ceil_offset to represent "unset" and "/". It's just a question of whether they should be -1 and 0 or 0 and 1.

--David

Johannes Schindelin wrote:

Show 17 quoted lines

> Hi,
> 
> On Thu, 15 May 2008, Johannes Sixt wrote:
> 
>> +             do { } while (offset > ceil_offset && cwd[--offset] != '/');
> 
> You probably meant to remove the "do { }", and have an own line
> 
>                         ; /* do nothing */
> 
> but for the rest, I agree that it is easier on the eye (particularly the
> off-by-one issue, which is always a problem for this developer to get
> right; avoiding it is therefore the better option).
> 
> Ciao,
> Dscho
>