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. > 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. Other than that, the patch looks good to me. Thanks, Corinna -- Corinna Vinschen Please, send mails regarding Cygwin to Cygwin Project Co-Leader cygwin AT cygwin DOT com Red Hat