Re: [PATCH v2] correct verify_path for Windows

[Alex Riesen <raa.lkml@gmail.com>]

Dmitry Potapov, Sun, Oct 12, 2008 15:50:48 +0200:
> On Sun, Oct 12, 2008 at 12:58:52AM +0200, Alex Riesen wrote:
> > 2008/10/11 Dmitry Potapov <dpotapov@gmail.com>:
> > >> > +   /* On Windows, file names are case-insensitive */
> > >> > +   case 'G':
> > >> > +           if ((rest[1]|0x20) != 'i')
> > >> > +                   break;
> > >> > +           if ((rest[2]|0x20) != 't')
> > >> > +                   break;
> > >>
> > >> We have tolower().
> > >
> > > I am aware of that, but I am not sure what we gain by using it. It seems
> > > it makes only code bigger and slow.
> > 
> > It does? Care to look into git-compat-util.h?
> 
> As a matter of fact, I did, and I see the following:
> 
>   #define sane_istest(x,mask) ((sane_ctype[(unsigned char)(x)] & (mask)) != 0)
>   #define tolower(x) sane_case((unsigned char)(x), 0x20)
> 
>   static inline int sane_case(int x, int high)
>   {
>   	if (sane_istest(x, GIT_ALPHA))
>   		x = (x & ~0x20) | high;
>   	return x;
>   }
> 
> So, it looks like an extra look up and an extra comparison here.

Does not look like much more code. But:

> > BTW, is it such a critical path?
> 
> I am not sure whether it is critical or not. It is called for each
> name in path. So, if you have a long path, it may be called quite a
> few times per a single path. Also, some operation such 'git add' can
> call verify_path() more than once (IIRC, it was called thrice per each
> added file). But I have no numbers to tell whether it is noticeable or
> not.

I looked at the callers (briefly). Performance could be a problem: add
and checkout can work with real big file lists and long pathnames.
So ok, than. It is critical.