Re: [PATCH 08/14] help.c: Fix detection of custom merge strategy on cygwin
- From
Ramsay Jones <ramsay@ramsay1.demon.co.uk>
- Date
- Dec 18, 2010, 20:41 UTC
- Message-ID
- <4D0D1C79.4070203@ramsay1.demon.co.uk>
- In-Reply-To
- <201012172246.45038.j6t@kdbg.org>
Johannes Sixt wrote:
Show 31 quoted lines
> On Donnerstag, 16. Dezember 2010, Ramsay Jones wrote:
>> Johannes Sixt wrote:
>>> On Dienstag, 14. Dezember 2010, Ramsay Jones wrote:
>>>> @@ -126,7 +126,10 @@ static int is_executable(const char *name)
>>>> !S_ISREG(st.st_mode))
>>>> return 0;
>>>>
>>>> -#ifdef WIN32
>>>> +#if defined(WIN32) || defined(__CYGWIN__)
>>>> +#if defined(__CYGWIN__)
>>>> +if ((st.st_mode & S_IXUSR) == 0)
>>>> +#endif
>>>> { /* cannot trust the executable bit, peek into the file instead */
>>>> char buf[3] = { 0 };
>>>> int n;
>>> Do you gain a lot by this extra condition? Wouldn't
>>>
>>> -#ifdef WIN32
>>> +#if defined(WIN32) || defined(__CYGWIN__)
>>>
>>> be sufficient?
>> Yes, that would be sufficient. No, I probably don't gain a great deal
>> (but I have *not* timed it), since the number of files that are tested
>> by is_executable() is fairly low anyway since they are already filtered
>> by a filename prefix (eg. git-merge-).
>>
>> However, if the executable bit is set, then executing the WIN32 code
>> block is wasted effort (we already know the answer), so why bother?
>
> It would have made to code a bit easier to read with one less #if defined(),
> but it's not a big deal.Yep, I did consider this:
-#idef WIN32 +#if defined(WIN32) || defined(__CYGWIN__) +if ((st.st_mode & S_IXUSR) == 0)
but chickened out! ;-) (No, I didn't even test it) I decided to be conservative here ...
ATB, Ramsay Jones