RFR (S) 8232168: Fix non wide char canonicalization on Windows
christoph.langer at sap.com
Wed Oct 16 07:21:09 UTC 2019
this item is really on the edge between core-libs and hotspot. So I'm including both lists now.
The canonicalize() method is implemented in libjava, for both Unix and Windows. For Unix, it is used from hotspot, libjava (the file system implementations) and libinstrument. But for Windows, canonicalize isn't actually used at all by libjava any more after JDK-8190737  since the WinNTFileSystem implementation uses wcanonicalize now.
Maybe this calls out for some further refactoring. One could move canonicalize completely into hotspot and use it from there. That would appear sensible to me but I don't know how the policy is for exporting methods from hotspot and consuming them in libjava. Alternatively, they could be kept at libjava but one has to know/keep in mind, that it is not used by the Windows java classlib implementation. If you say that it's the right way to go to move canonicalize into hotspot, I'd volunteer to bring this refactoring forward. Let me know.
But, apart from that possible refactoring, Ralf's change seems completely right to me. Also, using the hotspot test to verify the fix appears right. This test is a place where the functionality of Windows canonicalize is stressed and you won't find so many of them...
This fix seems a logical followup for JDK-8190737  and JDK-8191521 .
> On 15/10/2019 16:19, Schmelter, Ralf wrote:
> > Hello,
> > this is a small change which fixes the some problems with the canonicalize()
> method in canonicalize_md.c.
> Can you split out the changes to the java.io into a separate issue and
> bring them to core-libs-dev to discuss? I suspect the changes will
> require a lot of review cycles.
> > Currently it lets the wcanonicalize() method do the work for path lengths >
> MAX_PATH. This change always calls the wcanonicalize() method, since the
> old implementation doesn't work for all path lengths <= MAX_PATH. In
> addition the canonicalizeWithPrefix() method has been removed, since it
> seems to be not used anymore.
> > Bug report: https://bugs.openjdk.java.net/browse/JDK-8232168
> > Webrev:
> > Best regards,
> > Ralf
More information about the hotspot-runtime-dev