Re: [PATCH v2 2/9] setup_git_directory(): use is_dir_sep() helper

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Mar 3, 2017, 11:16 UTC
Message-ID: <alpine.DEB.2.20.1703031212280.3767@virtualbox>
In-Reply-To: <20170303033714.5h7qrq57hdqwoi3y@sigill.intra.peff.net>

Hi Peff,

On Thu, 2 Mar 2017, Jeff King wrote:

Show 22 quoted lines

> On Fri, Mar 03, 2017 at 03:04:07AM +0100, Johannes Schindelin wrote:
> 
> > It is okay in practice to test for forward slashes in the output of
> > getcwd(), because we go out of our way to convert backslashes to
> > forward slashes in getcwd()'s output on Windows.
> > 
> > Still, the correct way to test for a dir separator is by using the
> > helper function we introduced for that very purpose. It also serves as
> > a good documentation what the code tries to do (not "how").
> 
> Makes sense, but...
> 
> > @@ -910,7 +910,8 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
> >  			return setup_bare_git_dir(&cwd, offset, nongit_ok);
> >  
> >  		offset_parent = offset;
> > -		while (--offset_parent > ceil_offset && cwd.buf[offset_parent] != '/');
> > +		while (--offset_parent > ceil_offset &&
> > +		       !is_dir_sep(dir->buf[offset_parent]));
> 
> What is "dir"? I'm guessing this patch got reordered and it should stay
> as cwd.buf?

Oh drats. Usually I do a final `git rebase -x "make test" upstream/master` run before submitting, but I was really, really tired by the end of that stretch.

Thanks for being thorough (and I fixed it, of course), Dscho