Re: [PATCH v2] correct verify_path for Windows

From: Alex Riesen <raa.lkml@gmail.com>
Date: Oct 11, 2008, 22:58 UTC
Message-ID: <81b0412b0810111558vb69be00if4842fa91d777c3b@mail.gmail.com>
In-Reply-To: <20081011163310.GZ21650@dpotapov.dyndns.org>

2008/10/11 Dmitry Potapov <dpotapov@gmail.com>:

Show 11 quoted lines

>> > +   /* 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 to readability, I don't see much
> improvement... Isn't obvious what this code does, especially with the
> above comment?

You want to seriously argue that "a | 0x20" is as readable as "tolower(a)"? For the years to come? With a person who does not even know what ASCII is? Ok, I'm exaggerating. But the point is: it is not us who will be reading the code. And even if they do this just to remove Windows quirks it is well worth to use a bit more of english language so that they don't need a second look. As to comment: it is just additional info. It can't be checked by compiler if you make and accidental typo in your code (like, for example, accidentally putting an extra pipe in that expression, should happen to that emacs users from time to time).

BTW, is it such a critical path? Can't the code be unified and do without #ifdef?