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 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


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