Re: [PATCH 08/14] help.c: Fix detection of custom merge strategy on cygwin

[Johannes Sixt <j6t@kdbg.org>]

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.

-- Hannes