Re: Patch: Unimplemented functions in natFileWin32.cc

[Tom Tromey <tromey at redhat dot com>]

>>>>> "Ranjit" == Ranjit Mathew <rmathew at hotmail dot com> writes:

Ranjit>     This patch fleshes out the unimplemented portions of 
Ranjit> "java/io/natFileWin32.cc" so that methods like File.listRoots( ),
Ranjit> File.createNewFile( ), etc. start working on Win32.

This looks fine.  I'm checking it in to 3.3 and 3.4.  I didn't really
look very closely at the Win32 native bits; instead I'm taking your
word for it.  (I couldn't meaningfully review those anyway.)

Note that your patch was wrapped in a couple places.  Something wrong
with the mailer?  No biggie; I had to edit it a little to get it to
apply.

Ranjit> +        if (plen > 1 && p.charAt (plen - 1) == separatorChar)
Ranjit> +          if (separatorChar == '\\' && plen == 3 && p.charAt (1) == ':')
Ranjit> +            ;
Ranjit> +          else
Ranjit> +            return p.substring (0, plen - 1);
Ranjit>  	else
Ranjit>  	  return p;

In a situation like this we prefer braces around the inner `if'
statement.  Also the empty `;' is weird; it is clearer to invert the
condition or to add a new block with a comment explaining what is
going on.

I rewrote it to this:

@@ -96,9 +96,13 @@
 
     if (dupIndex == -1)
       {
-        // Ignore trailing separator.
-        if (plen > 1 && p.charAt(plen - 1) == separatorChar)
-	  return p.substring(0, plen - 1);
+        // Ignore trailing separator (though on Windows "a:\", for
+        // example, is a valid and minimal path).
+        if (plen > 1 && p.charAt (plen - 1) == separatorChar)
+	  {
+	    if (! (separatorChar == '\\' && plen == 3 && p.charAt (1) == ':'))
+	      return p.substring (0, plen - 1);
+	  }
 	else
 	  return p;
       }

Tom