RFR: 8189953: FileHandler constructor throws NoSuchFileException with absolute path
Hi, Please find below a patch for: 8189953: FileHandler constructor throws NoSuchFileException with absolute path https://bugs.openjdk.java.net/browse/JDK-8189953 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8189953/webrev.00/ This is a windows only bug. The issue is caused by a change that happen somewhen between 8 and 9, where new File(new File("C:"),"Workspace") was fixed to return "C:Workspace" and not "C:\\Workspace". This uncovered a bug in the FileHandler::generate algorithm. The FileHandler::generate algorithm could arguably be improved by being entirely rewritten but I choose to keep the changes as minimal as I could. best regards, -- daniel
Hi Daniel, For more background on this issue I think you should link to this issue to the following: https://bugs.openjdk.java.net/browse/JDK-8153250 https://bugs.openjdk.java.net/browse/JDK-8130462 https://bugs.openjdk.java.net/browse/JDK-8189862 Thanks, Jason ________________________________________ From: core-libs-dev <core-libs-dev-bounces@openjdk.java.net> on behalf of Daniel Fuchs <daniel.fuchs@oracle.com> Sent: Thursday, November 2, 2017 11:47 AM To: core-libs-dev Subject: RFR: 8189953: FileHandler constructor throws NoSuchFileException with absolute path Hi, Please find below a patch for: 8189953: FileHandler constructor throws NoSuchFileException with absolute path https://bugs.openjdk.java.net/browse/JDK-8189953 webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8189953/webrev.00/ This is a windows only bug. The issue is caused by a change that happen somewhen between 8 and 9, where new File(new File("C:"),"Workspace") was fixed to return "C:Workspace" and not "C:\\Workspace". This uncovered a bug in the FileHandler::generate algorithm. The FileHandler::generate algorithm could arguably be improved by being entirely rewritten but I choose to keep the changes as minimal as I could. best regards, -- daniel
On 02/11/2017 17:55, Jason Mehrens wrote:
Hi Daniel,
For more background on this issue I think you should link to this issue to the following:
https://bugs.openjdk.java.net/browse/JDK-8153250 https://bugs.openjdk.java.net/browse/JDK-8130462 https://bugs.openjdk.java.net/browse/JDK-8189862
Oh, thanks Jason! That is great :-) I have linked the issues. best regards, -- daniel
Hi Daniel, On 11/2/17 9:47 AM, Daniel Fuchs wrote:
Hi,
Please find below a patch for: 8189953: FileHandler constructor throws NoSuchFileException with absolute path https://bugs.openjdk.java.net/browse/JDK-8189953
webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8189953/webrev.00/
This looks okay. A minor comment: I think line 695-709 can be made cleaner and easier to read and e.g. if (word.length() >0) { String n = word.toString(); if (result ==null) { result = prev !=null ? prev.resolveSibling(n) : Paths.get(n); }else { Path p = prev !=null ? prev.resolveSibling(n) : Paths.get(n); result = result.resolve(p); } }else if (result ==null) { result = Paths.get(""); } if (path.getRoot() ==null) return result.toFile(); else return path.getRoot().resolve(result).toFile(); Mandy
Hi Mandy, Thanks for the review - and thanks for reminding me about @modules java.logging/java.util.logging:open I have a new webrev that incorporates your suggestions. I have also tried to reduce the long lines (I recently switched to a new IDE whose default config make them more difficult to spot). http://cr.openjdk.java.net/~dfuchs/webrev_8189953/webrev.01 best regards, -- daniel On 09/11/2017 17:41, mandy chung wrote:
Hi Daniel,
On 11/2/17 9:47 AM, Daniel Fuchs wrote:
Hi,
Please find below a patch for: 8189953: FileHandler constructor throws NoSuchFileException with absolute path https://bugs.openjdk.java.net/browse/JDK-8189953
webrev: http://cr.openjdk.java.net/~dfuchs/webrev_8189953/webrev.00/
This looks okay.
A minor comment: I think line 695-709 can be made cleaner and easier to read and
e.g.
if (word.length() >0) { String n = word.toString(); if (result ==null) { result = prev !=null ? prev.resolveSibling(n) : Paths.get(n); }else { Path p = prev !=null ? prev.resolveSibling(n) : Paths.get(n); result = result.resolve(p); } }else if (result ==null) { result = Paths.get(""); } if (path.getRoot() ==null) return result.toFile(); else return path.getRoot().resolve(result).toFile();
Mandy
On 11/9/17 11:20 AM, Daniel Fuchs wrote:
Hi Mandy,
Thanks for the review - and thanks for reminding me about @modules java.logging/java.util.logging:open
I have a new webrev that incorporates your suggestions. I have also tried to reduce the long lines (I recently switched to a new IDE whose default config make them more difficult to spot).
Looks good. thanks Mandy
participants (3)
-
Daniel Fuchs
-
Jason Mehrens
-
mandy chung