Hi,

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 [0] 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 [0] and JDK-8191521 [1].

Best regards
Christoph

[0] https://bugs.openjdk.java.net/browse/JDK-8190737
[1] https://bugs.openjdk.java.net/browse/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.
> 
> -Alan
> 
> 
> > 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:
> http://cr.openjdk.java.net/~rschmelter/webrevs/8232168/webrev.0/
> >
> > Best regards,
> > Ralf

Reply via email to