[ruby-core:35001] file_expand_path performing unneeded IO operations on native Windows

Hello,

To follow up what I asked in [ruby-core:34991]:
http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-core/34991

I've been digging into aspects of file and path operations in Windows,
particularly this case is file_expand_path, defined in file.c:2864

Around line 3094, it allocates WIN32_FIND_DATA structure which is
filed in 3113 and compared it's mode against S_ISLNK

There are a few __CYGWIN__ blocks around this process, and is clear
that the purpose of obtain this state information is for Cygwin and
it's fake symlink functionality:

	if (lstat(buf, &st) == 0 && S_ISLNK(st.st_mode)) {
	    is_symlink = 1;
	    if (len > 4 && STRCASECMP(p + len - 4, ".lnk") != 0) {
		lnk_added = 1;
	    }
	}


What is not clear, however, is why this check is performed for all
Windows versions, knowing that st.st_mode will never contain symlink
information on it's current state and the logic seems centered around
cygwin.

This check generates a FindFirstFileA around line 3148:

	h = FindFirstFile(b, &wfd);
	if (h != INVALID_HANDLE_VALUE) {
	    FindClose(h);
	    len = strlen(wfd.cFileName);


Just to obtain the filename len used by the __CYGWIN__ ifdef block,
but never used on Windows itself.

So a simple File.expand_path('foo.rb') results in the following 2 API calls:

FindFirstFileA (
"C:/Users/Luis/Projects/oss/oci/rubyinstaller/sandbox/foo.rb",
0x0028c8e0 )
FindClose ( 0x00865e58 )

Which on Linux doesn't (strace):

Process 24459 attached - interrupt to quit
read(0, "\r", 8192)                     = 1
write(1, "\n", 1)                       = 1
rt_sigprocmask(SIG_BLOCK, [INT], [], 8) = 0
ioctl(0, SNDCTL_TMR_TIMEBASE or TCGETS, {B38400 opost isig -icanon
-echo ...}) = 0
ioctl(0, SNDCTL_TMR_STOP or TCSETSW, {B38400 opost isig icanon echo ...}) = 0
ioctl(0, SNDCTL_TMR_TIMEBASE or TCGETS, {B38400 opost isig icanon echo ...}) = 0
rt_sigprocmask(SIG_SETMASK, [], NULL, 8) = 0
time(NULL)                              = 1296377546
getcwd("/home/user/sandbox", 200)       = 19
write(1, "=> \"/home/user/sandbox/foo.rb\"\n", 31) = 31

===

Ruby 1.9.x expands every file prior adding it to the $LOADED_FEATURES
array, so loading environments like Rails means lot of operations in
path expansion that are hitting the disk.

While I'm familiarizing with the codebase, some questions raises which
I would like to get a better understanding prior introducing
modifications.

Are my above conclusions incorrect? I'm missing something?

The following trivial patch removes the API calls for native Windows,
while remains checking for them on Cygwin:

diff --git a/file.c b/file.c
index 00a2a0c..1a7d289 100644
--- a/file.c
+++ b/file.c
@@ -3091,13 +3091,13 @@ file_expand_path(VALUE fname, VALUE dname, int
abs_mode, VALUE result)
     }
     if (p == skiproot(buf) - 1) p++;

+#ifdef __CYGWIN__
 #if USE_NTFS
     *p = '\0';
     if ((s = strrdirsep(b = buf)) != 0 && !strpbrk(s, "*?")) {
        size_t len;
        WIN32_FIND_DATA wfd;
        HANDLE h;
-#ifdef __CYGWIN__
 #ifdef HAVE_CYGWIN_CONV_PATH
        char *w32buf = NULL;
        const int flags = CCP_POSIX_TO_WIN_A | CCP_RELATIVE;

===

And it passes test_file.rb and test_file_exhaustive.rb

Can someone provide details on Cygwin's behavior?

Thank you.
-- 
Luis Lavena
AREA 17
-
Perfection in design is achieved not when there is nothing more to add,
but rather when there is nothing more to take away.
Antoine de Saint-Exup駻y