Re: [PATCH 08/14] help.c: Fix detection of custom merge strategy on cygwin
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Dec 17, 2010, 21:46 UTC
- Message-ID
- <201012172246.45038.j6t@kdbg.org>
- In-Reply-To
- <4D0A80C5.4020003@ramsay1.demon.co.uk>
On Donnerstag, 16. Dezember 2010, Ramsay Jones wrote:
Show 29 quoted lines
> 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