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

[Brandon Williams <bmwill@google.com>]

On 01/04, Jeff King wrote:
> On Wed, Jan 04, 2017 at 07:56:02AM +0100, Torsten Bögershausen wrote:
> 
> > 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. :)

I agree that we should have similar guarantees as the OS provides,
especially if the OS already has MAXSYMLINKS defined.  What then, should
the fall back value be if the OS doesn't have this defined?  5 like we
have done historically, or something around the 20-30 range as Torsten
suggests?

-- 
Brandon Williams