Hi Oleg,

> > Regarding the motivation, i.e.: What is wrong with the existing code?
> 
> It is slow, specifically GetFinalPathNameByHandleA. The patch gives a
> speedup of up to 20% when searching a directory containing a GCC
> checkout.

OK, good to hear that you have actually measured it.

> > The documentation of GetFinalPathNameByHandleA says that it resolves 
> > symbolic links;
> > the documentation of GetFileInformationByHandleEx doesn't.
> 
> I tested the current implementation of stat on a symlink. It does not
> get the target filename but uses the passed-in filename of the symlink
> itself.
> 
> I fixed it in v2 of the patch. It now always gets the target filename
> using GetFileInformationByHandleEx. It handles symlinks just as well
> as GetFinalPathNameByHandleA and is faster on top of that.
> 
> $ ln -s stat-exec.exe stat-link
> 
> before patch:
> $ ./stat-exec.exe stat-link
> file: stat-link
> stat : mode=0100666 executable=no
> fstat: mode=0100777 executable=yes
> 
> after patch:
> $ ./stat-exec.exe stat-link
> file: stat-link
> stat : mode=0100777 executable=yes
> fstat: mode=0100777 executable=yes

Oh, very nice! Now your patch does two things:
  1) It corrects the behaviour on symbolic links.
  2) It ptovides a speedup.

> I fixed it in v2 of my patch.

The 'name_known' variable is not needed; a simple 'else' does it, as before.

I split your patch into two patches (1 and 2), and committed them separately.

Many thanks!

Bruno




Reply via email to