On 26/06/2012 07:00, Xueming Shen wrote:
On 6/25/12 10:58 PM, Xueming Shen wrote:
Hi,
While I still believe that case-insensitive is the right choice for
File/Path on MacOSX, it is
suggested that we might want to be a little conservative in this
patch, with the assumption
that this patch will be backport to 7u release after being baked in
jdk8 for a while (given
Apple's JDK6 is case sensitive for File, it might be too much for a
update releases to go
with two in-compatible changes, case sensitive and hash32).
So here is the webrev for a strip-down version from the original
patch, in which it only
targets the nfd/nfc issue raised in 7130915 and 7168427. The proposed
changes for
case insensitive compare and hash32 for both java.io.File and
java.nio.file.Path are
removed.
http://cr.openjdk.java.net/~sherman/7130915_7168427/webrev
(The webrev for the "full" version has been moved to
http://cr.openjdk.java.net/~sherman/7130915_7168427/webrev_full)
Thanks,
-Sherman
I had to dig up the File Systems, Unicode, and Normalization
presentation [1] before reviewing this, it's been a while.
I think the changes for java.io for fine, I just worry that there may be
a few odd cases where it will be different to Apple JDK6. I looked
through the macosx-port/macosx-port/jdk forest and there is one patch
from Apple that does the NFC->NFD conversation. I don't get that patch
because you say that the syscalls handle NFC okay. In your changes then
you normalize when decoding the file names to Strings, which seems right
but that seems to be completely missing from the changes that Apple
brought over. The only minor comment is that we probably need to check
the return from core foundation functions such as CFStringCreateMutable
(this goes for the other native code too).
Adding the FileSystemProvider for MacOSX is great to see. The approach
is fine for but is somewhat inefficient when ->NFD or ->NFC is needed.
One inefficiency that can be fixed is in sun.nio.fs.MacOSXFileSystem.
normalizeJavaPath then you can eliminate the second call to toCharArray.
I think the new methods on sun.no.fs.UnixFileSystem should be given
comments so that it's clear whether they should be overridden (the Unix*
provider tends to be starting point for most ports). A minor comment is
that MacOSXNativeDispatcher has a bunch of blank lines at the end, also
I think "static final" is more normal than "final static". At some point
I think we should re-write MacOSXNativeDispatcher.normalizepath so that
it deosn't require the critical section or malloc as in this area we try
to keep the native methods to a bare minimum.
Do the tests assume they are run on HFS? Just wondering if you you need
to look at the FileStore name/type to check. Some of variable namesa bit
non-standard, very C like. Minor nit is that the copyright in the header
isn't right in the tests. Also the tests list 2 CRs whereas I assume
this is one CR (with the other closed as a dup).
On case sensitive vs. insensitive then I think it should be insensitive
as per the first round but that would be a significant change given that
Apple's JDK does not appear to have changed File.equals. Maybe we should
think about this again once these changes are in 8.
-Alan
[1]
http://developers.sun.com/global/products_platforms/solaris/reference/presentations/IUC29-FileSystems.pdf
