The semantics are the same, except that I filter out directories.
Granted, I did change how those semantics were expressed, but my
preference is for clearer code over minimal patch size.

Anyway, it's not like that function is a paragon of clarity.  I was tempted
to rewrite the whole thing but resisted that temptation.

That said, I don't care either way, but it does seem like a useful
check to include.

- Eric

On Thu, Jun 26, 2008 at 3:25 PM, Johannes Schindelin
<[EMAIL PROTECTED]> wrote:
> Hi,
>
> On Thu, 26 Jun 2008, Eric Raible wrote:
>
>> diff --git a/compat/mingw.c b/compat/mingw.c
>> index 5499a95..ab5d966 100644
>> --- a/compat/mingw.c
>> +++ b/compat/mingw.c
>> @@ -523,9 +523,10 @@ static char *lookup_prog(const char *dir, const char 
>> *cmd,
>> int isexe, int exe_on
>>       if (!isexe && access(path, F_OK) == 0)
>>               return xstrdup(path);
>>       path[strlen(path)-4] = '\0';
>> -     if ((!exe_only || isexe) && access(path, F_OK) == 0)
>> -             return xstrdup(path);
>> -     return NULL;
>> +     if ((exe_only && !isexe) || access(path, F_OK) ||
>> +             (GetFileAttributes(path) & FILE_ATTRIBUTE_DIRECTORY))
>> +             return NULL;
>> +     return xstrdup(path);
>
> It would be much more logical to keep the original semantics, and do this
> instead of your patch:
>
> -       if ((!exe_only || isexe) && access(path, F_OK) == 0)
> +       if ((!exe_only || isexe) && access(path, F_OK) == 0 &&
> +                       !(GetFileAttributes(path) &
> +                               FILE_ATTRIBUTE_DIRECTORY))
>
> Ciao,
> Dscho
>
>

Reply via email to