Re: [PATCH 2/2] ntdll: Check for case-insensitive volumes.

[Alexandre Julliard]

Charles Davis <cdavis(a)mymail.mines.edu> writes:
> @@ -1828,13 +2054,24 @@ static NTSTATUS find_file_in_dir( char *unix_name, int pos, const WCHAR *name, i
>      if (ret >= 0 && !used_default)
>      {
>          unix_name[pos + ret] = 0;
> -        if (!stat( unix_name, &st ))
> +        /* when checking case on a case insensitive but case preserving
> +         * fs, don't even bother doing the stat(), so we don't match the 
> +         * file case-insensitively.
> +         */
> +        if ((case_sensitive || !check_case) && !stat( unix_name, &st ))
>          {
>              if (is_win_dir) *is_win_dir = is_same_file( &windir, &st );
>              return STATUS_SUCCESS;
>          }

This still doesn't make sense. There's no reason to ever skip the stat.
> @@ -2455,6 +2713,10 @@ static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer
>          const WCHAR *end, *next;
>          int is_win_dir = 0;
>
> +        unix_name[pos] = 0;
> +        stat( unix_name, &st );
> +        unix_name[pos] = '/';

You can't add extra stat calls to the normal path. This is supposed to
be an optimization.

-- 
Alexandre Julliard
julliard(a)winehq.org

[PATCH 2/2] ntdll: Check for case-insensitive volumes.

[Charles Davis]

On 10/18/10 8:28 AM, Alexandre Julliard wrote:
> Charles Davis <cdavis(a)mymail.mines.edu> writes:
> > @@ -1828,13 +2054,24 @@ static NTSTATUS find_file_in_dir( char *unix_name, int pos, const WCHAR *name, i
> >      if (ret >= 0 && !used_default)
> >      {
> >          unix_name[pos + ret] = 0;
> > -        if (!stat( unix_name, &st ))
> > +        /* when checking case on a case insensitive but case preserving
> > +         * fs, don't even bother doing the stat(), so we don't match the 
> > +         * file case-insensitively.
> > +         */
> > +        if ((case_sensitive || !check_case) && !stat( unix_name, &st ))
> >          {
> >              if (is_win_dir) *is_win_dir = is_same_file( &windir, &st );
> >              return STATUS_SUCCESS;
> >          }
>
> This still doesn't make sense. There's no reason to ever skip the stat.
> When I wrote that, I was thinking, "If we do the stat() on a
> case-insensitive file system, it will succeed even if the filename has
> the wrong case. We don't want that when we're checking case, so we may
> as well read the dirents to see if the file exists with the right case."
> Is there something I'm missing? Would check_case even matter when
> case_sensitive is FALSE?
> > @@ -2455,6 +2713,10 @@ static NTSTATUS lookup_unix_name( const WCHAR *name, int name_len, char **buffer
> >          const WCHAR *end, *next;
> >          int is_win_dir = 0;
> >
> > +        unix_name[pos] = 0;
> > +        stat( unix_name, &st );
> > +        unix_name[pos] = '/';
>
> You can't add extra stat calls to the normal path. This is supposed to
> be an optimization.
> Yeah, you're right, as usual. I'll fix this in try 4.

Chip

[PATCH 2/2] ntdll: Check for case-insensitive volumes.

[Alexandre Julliard]

Charles Davis <cdavis(a)mymail.mines.edu> writes:
> When I wrote that, I was thinking, "If we do the stat() on a
> case-insensitive file system, it will succeed even if the filename has
> the wrong case. We don't want that when we're checking case, so we may
> as well read the dirents to see if the file exists with the right case."
> Is there something I'm missing? Would check_case even matter when
> case_sensitive is FALSE?

If the file system is not case sensitive then there's no case to check.

-- 
Alexandre Julliard
julliard(a)winehq.org

[PATCH 2/2] ntdll: Check for case-insensitive volumes.

[Charles Davis]

On 10/18/10 10:57 AM, Alexandre Julliard wrote:
> Charles Davis <cdavis(a)mymail.mines.edu> writes:
> > When I wrote that, I was thinking, "If we do the stat() on a
> > case-insensitive file system, it will succeed even if the filename has
> > the wrong case. We don't want that when we're checking case, so we may
> > as well read the dirents to see if the file exists with the right case."
> > Is there something I'm missing? Would check_case even matter when
> > case_sensitive is FALSE?
>
> If the file system is not case sensitive then there's no case to check.
> Only if the file system doesn't preserve case. Most modern
> case-insensitive file systems (FAT, NTFS, HFS) preserve case. CIOPFS
> preserves case if it's allowed to write xattrs to the underlying FS.
> This is why I even had the case-preserving handling in there in the
> first place.

So, if I understand correctly, you think that, if we determine that file
system is case-insensitive, we should also assume it doesn't preserve
case. Is that correct?

Chip

[PATCH 2/2] ntdll: Check for case-insensitive volumes.

[Alexandre Julliard]

Charles Davis <cdavis(a)mymail.mines.edu> writes:
> Only if the file system doesn't preserve case. Most modern
> case-insensitive file systems (FAT, NTFS, HFS) preserve case. CIOPFS
> preserves case if it's allowed to write xattrs to the underlying FS.
> This is why I even had the case-preserving handling in there in the
> first place.
>
> So, if I understand correctly, you think that, if we determine that file
> system is case-insensitive, we should also assume it doesn't preserve
> case. Is that correct?

No. The check_case flag is used to specify Unix semantics, but Unix
semantics on a case-insensitive filesystem are just the same as Windows.
It doesn't make sense to try to be more case sensitive than the
filesystem.

-- 
Alexandre Julliard
julliard(a)winehq.org