сб, 5 сент. 2026 г. в 15:48, Bruno Haible <[email protected]>: > > Hi Oleg, > > Thanks for the proposed patch.
Thanks for the fast review. > 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. > > GetFinalPathNameByHandle canonicalizes the path of an open handle, which > > costs a round trip into the file system for every file that is stat'ed. > > Only the file name suffix is needed here, and GetFileInformationByHandleEx > > with argument FileNameInfo answers from the handle itself. > > If I understand it correctly, you say that the existing code is slower > than needed. Yes, exactly. > I'm more worried about correctness than about speed. > > What about files that are native Windows symlinks, created through 'mklink' > [1]? > I mean, if we have a link > foo -> bar.exe > the function stat() [as opposed to lstat()] is supposed to find the suffix > ".exe" and thus return an "executable bit" in the mode. > 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 > Then, regarding the patch: I think it uses the WideCharToMultiByte function > [2] > incorrectly. A second argument of 0 is almost never what you want. > > Actually, you could remove that WideCharToMultiByte call. Testing for a .exe > suffix should be possible at the WCHAR[] level already. > > Bruno I fixed it in v2 of my patch. > [1] > https://learn.microsoft.com/en-us/windows-server/administration/windows-commands/mklink > [2] > https://learn.microsoft.com/en-us/windows/win32/api/stringapiset/nf-stringapiset-widechartomultibyte > > >
