Re: [PATCH v4 0/5] road to reentrant real_path

From: Jeff King <peff@peff.net>
Date: Jan 4, 2017, 07:01 UTC
Message-ID: <20170104070107.huse2a6thz737epv@sigill.intra.peff.net>
In-Reply-To: <3f9a530c-402f-f276-4721-fa6a8a6fef41@web.de>

On Wed, Jan 04, 2017 at 07:56:02AM +0100, Torsten Bögershausen wrote:

Show 19 quoted lines

> On 04.01.17 01:48, Jeff King wrote:
> > On Tue, Jan 03, 2017 at 11:09:18AM -0800, Brandon Williams wrote:
> > 
> >> Only change with v4 is in [1/5] renaming the #define MAXSYMLINKS back to
> >> MAXDEPTH due to a naming conflict brought up by Lars Schneider.
> > 
> > Hmm. Isn't MAXSYMLINKS basically what you want here, though? It what's
> > what all other similar functions will be using.
> > 
> > The only problem was that we were redefining the macro. So maybe:
> > 
> >   #ifndef MAXSYMLINKS
> >   #define MAXSYMLINKS 5
> >   #endif
> > 
> > would be a good solution?
> Why 5  ? (looking at the  20..30 below)
> And why 5 on one system and e.g. on my Mac OS
> #define MAXSYMLINKS     32

I mentioned "5" because that is the current value of MAXDEPTH. I do think it would be reasonable to bump it to something higher.

> Would the same value value for all Git installations on all platforms make sense?
> #define GITMAXSYMLINKS 20

I think it's probably more important to match the rest of the OS, so that open("foo") and realpath("foo") behave similarly on the same system. Though I think even that isn't always possible, as the limit is dynamic on some systems.

I think the idea of the 20-30 range is that it's small enough to catch an infinite loop quickly, and large enough that nobody will ever hit it in practice. :)

-Peff