Re: [PATCH 2/2] ntdll: Check for case-insensitive volumes. (resend try 4)

[Alexandre Julliard]

Charles Davis <cdavis(a)mymail.mines.edu> writes:
> If the volume is indeed case insensitive, we skip searching through the
> whole directory ourselves, and just stat() each portion of the path.

You still have extra stat() calls in the fast path, as well as some
unnecessary changes.

-- 
Alexandre Julliard
julliard(a)winehq.org

[PATCH 2/2] ntdll: Check for case-insensitive volumes. (resend try 4)

[Charles Davis]

On 10/27/10 3:49 AM, Alexandre Julliard wrote:
> Charles Davis <cdavis(a)mymail.mines.edu> writes:
> > If the volume is indeed case insensitive, we skip searching through the
> > whole directory ourselves, and just stat() each portion of the path.
>
> You still have extra stat() calls in the fast path,
> I couldn't get rid of all of them. The one in lookup_unix_name() has to
> stay AFAIK; otherwise SFNs break in some weird way that I can't seem to
> figure out. (kernel32's path test, for example, has 4 failures, all of
> them related to short paths.)
> as well as some
> unnecessary changes.
> Which ones?

Perhaps you're referring to the changes to get_redirect_path() and
match_redirect(). My upcoming try 5 forces check_case to false when
calling get_redirect_path() on a case-insensitive FS, instead of
threading that bit through. Is that better? Or do you want me to remove
that altogether?

Chip

[PATCH 2/2] ntdll: Check for case-insensitive volumes. (resend try 4)

[Alexandre Julliard]

Charles Davis <cdavis(a)mymail.mines.edu> writes:
> On 10/27/10 3:49 AM, Alexandre Julliard wrote:
> > You still have extra stat() calls in the fast path,
> > I couldn't get rid of all of them. The one in lookup_unix_name() has to
> > stay AFAIK; otherwise SFNs break in some weird way that I can't seem to
> > figure out. (kernel32's path test, for example, has 4 failures, all of
> > them related to short paths.)

That's of course the sign of a bigger problem, namely that you don't
handle short paths at all on case insensitive file systems.

If the tests didn't catch this sooner, it means that you should probably
first write more tests with various case and short names combinations.

-- 
Alexandre Julliard
julliard(a)winehq.org

[PATCH 2/2] ntdll: Check for case-insensitive volumes. (resend try 4)

[Charles Davis]

On 10/27/10 10:48 AM, Alexandre Julliard wrote:
> Charles Davis <cdavis(a)mymail.mines.edu> writes:
> > On 10/27/10 3:49 AM, Alexandre Julliard wrote:
> > > You still have extra stat() calls in the fast path,
> > > I couldn't get rid of all of them. The one in lookup_unix_name() has to
> > > stay AFAIK; otherwise SFNs break in some weird way that I can't seem to
> > > figure out. (kernel32's path test, for example, has 4 failures, all of
> > > them related to short paths.)
>
> That's of course the sign of a bigger problem, namely that you don't
> handle short paths at all on case insensitive file systems.
> I just ran the tests on Linux, and oddly enough they don't fail without
> the extra stat() in lookup_unix_name(). So AFAIK it's peculiar to Mac
> OS. What's different about Wine on Linux vs. Wine on Mac OS that could
> cause this?

Chip