From: Johannes Sixt <j6t@kdbg.org>
Date: Fri, 17 Dec 2010 21:46:44 GMT
Subject: Re: [PATCH 08/14] help.c: Fix detection of custom merge strategy on cygwin
Message-ID: <201012172246.45038.j6t@kdbg.org>
In-Reply-To: <4D0A80C5.4020003@ramsay1.demon.co.uk>

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