This is the mail archive of the cygwin-apps mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH 3/4] Make io_stream::exists() directory aware


On 29/11/2010 11:28, Corinna Vinschen wrote:
> On Nov 26 13:48, Jon TURNEY wrote:
>> At the moment io_stream::exists() returns FALSE for file:// paths
>> which refer to an existing directory.  Inconsistently, for
>> cygfile:// paths which refer to an existing directory, it returns
>> TRUE.
>>
>> Return a new state, IO_STREAM_EXISTS_DIRECTORY, to indicate if
>> pathname exists as a directory and update all uses appropriately
>>
>> Not sure if the use of access() in the legacy branch of
>> io_stream_cygfile::exists() is correct, looks like it's inverted
> 
> Indeed.

It's somewhat academic as I think io_stream_cygfile::exists() is never used at
the moment.

>> Not sure if current exists() implementation deals correctly when other
>> attributes are set for a file, e.g. FILE_ATTRIBUTE_COMPRESSED or
>> FILE_ATTRIBUTE_ENCRYPTED, since it checks attributes against an
>> expected value rather than checking for bits being set?
> 
> No, the original implementation certainly doesn't look quite right.
> 
>> @@ -196,11 +197,21 @@ io_stream_cygfile::exists (const std::string& path)
>>        mklongpath (wname, cygpath (normalise(path)).c_str (), len);
>>        DWORD attr = GetFileAttributesW (wname);
>>        if (attr != INVALID_FILE_ATTRIBUTES)
>> -	return 1;
>> +        return (attr & FILE_ATTRIBUTE_DIRECTORY) ? IO_STREAM_EXISTS_DIRECTORY : IO_STREAM_EXISTS_FILE;
>>      }
>>    else if (_access (cygpath (normalise(path)).c_str (), 0))
>> -    return 1;
>> -  return 0;
>> +    {
>> +      struct _stat s;
>> +      if (!_stat (cygpath (normalise(path)).c_str (), &s))
>> +        {
>> +          if (s.st_mode & S_IFDIR)
>> +            return IO_STREAM_EXISTS_DIRECTORY;
>> +
>> +          if (s.st_mode & S_IFREG)
>> +            return IO_STREAM_EXISTS_FILE;
>> +        }
>> +    }
>> +  return IO_STREAM_EXISTS_NO;
>>  }
> 
> I would prefer if you would use GetFileAttributesA here, just like the
> io_stream_file::exists method.  This also unifies testing the
> attributes.

Oh, I used _stat() in the !IsWindowsNT() case as MSDN tells me that
GetFileAttributes() isn't available prior to Win2K.  I guess I've been misled :-)

Updated patch attached

Attachment: 0001-Make-io_stream-exists-directory-aware.patch
Description: Text document


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]