RFR: 8377910: Minor cleanup of java/io/FileDescriptor/Sharing.java
Null checks potentially mask a more serious, fundamental problem and should be removed to allow a `NullPointerException` to be throw if the variable is `null`. ------------- Commit messages: - 8377910: Minor cleanup of java/io/FileDescriptor/Sharing.java Changes: https://git.openjdk.org/jdk/pull/29718/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=29718&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8377910 Stats: 7 lines in 1 file changed: 0 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/29718.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/29718/head:pull/29718 PR: https://git.openjdk.org/jdk/pull/29718
On Fri, 13 Feb 2026 17:14:31 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Null checks potentially mask a more serious, fundamental problem and should be removed to allow a `NullPointerException` to be throw if the variable is `null`.
It appears that the `null` checks in question were intended to match others elsewhere in the test where the checks are in a finally block and there are multiple variables among which not all might be `null` depending on which variables were initialized in the `try` block. ------------- PR Comment: https://git.openjdk.org/jdk/pull/29718#issuecomment-3898346004
Null checks potentially mask a more serious, fundamental problem and should be removed to allow a `NullPointerException` to be throw if the variable is `null`.
Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8377910: In class OpenClose make arrays local to run method and other instance variables final ------------- Changes: - all: https://git.openjdk.org/jdk/pull/29718/files - new: https://git.openjdk.org/jdk/pull/29718/files/9954cca5..9dd1613c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=29718&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29718&range=00-01 Stats: 7 lines in 1 file changed: 3 ins; 2 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/29718.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/29718/head:pull/29718 PR: https://git.openjdk.org/jdk/pull/29718
On Fri, 13 Feb 2026 17:49:29 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Null checks potentially mask a more serious, fundamental problem and should be removed to allow a `NullPointerException` to be throw if the variable is `null`.
Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
8377910: In class OpenClose make arrays local to run method and other instance variables final
test/jdk/java/io/FileDescriptor/Sharing.java line 370:
368: for(int i=0;i<numFiles;i++) { 369: fisArray[i].close(); 370: fosArray[i].close();
It might be clearer if you change fisArray and fosArray to be local to the run method. There is no reason for them to be fields in OpenClose. Making fd and done final would help too. I can't see from the bug report what failed but hopefully better output with the changes. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29718#discussion_r2805363903
On Fri, 13 Feb 2026 17:42:49 GMT, Alan Bateman <alanb@openjdk.org> wrote:
I can't see from the bug report what failed but hopefully better output with the changes.
It's not clear exactly what failed (in the parent task issue) but perhaps this will disambiguate the situation a bit. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29718#discussion_r2805373689
On Fri, 13 Feb 2026 17:45:13 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
test/jdk/java/io/FileDescriptor/Sharing.java line 370:
368: for(int i=0;i<numFiles;i++) { 369: fisArray[i].close(); 370: fosArray[i].close();
It might be clearer if you change fisArray and fosArray to be local to the run method. There is no reason for them to be fields in OpenClose. Making fd and done final would help too.
I can't see from the bug report what failed but hopefully better output with the changes.
I can't see from the bug report what failed but hopefully better output with the changes.
It's not clear exactly what failed (in the parent task issue) but perhaps this will disambiguate the situation a bit.
It might be clearer if you change fisArray and fosArray to be local to the run method. There is no reason for them to be fields in OpenClose. Making fd and done final would help too.
Done in 9dd1613. Note that this code has bad indentation but I am leaving that alone until the end. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29718#discussion_r2805387796
On Fri, 13 Feb 2026 17:48:58 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Note that this code has bad indentation but I am leaving that alone until the end.
Indentation fixed in [73d9eab](https://github.com/openjdk/jdk/pull/29718/commits/73d9aebadb13b417945553f850...). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29718#discussion_r2805714196
On Fri, 13 Feb 2026 17:52:48 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Null checks potentially mask a more serious, fundamental problem and should be removed to allow a `NullPointerException` to be throw if the variable is `null`.
Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
8377910: In class OpenClose make arrays local to run method and other instance variables final
test/jdk/java/io/FileDescriptor/Sharing.java line 380:
378: if (fd.valid()) { // fd should not be valid after first close() call 379: System.err.println("OpenClose: FileDescriptor shouldn't be valid"); 380: fail = true;
The failure reported in the parent issue occurs because `fd.valid()` is `true` at line 378. Code inspection alone does not reveal any reason for it. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29718#discussion_r2805600941
On Fri, 13 Feb 2026 17:52:48 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Null checks potentially mask a more serious, fundamental problem and should be removed to allow a `NullPointerException` to be throw if the variable is `null`.
Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
8377910: In class OpenClose make arrays local to run method and other instance variables final
Marked as reviewed by alanb (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/29718#pullrequestreview-3798972892
Null checks potentially mask a more serious, fundamental problem and should be removed to allow a `NullPointerException` to be throw if the variable is `null`.
Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision: 8377910: Fix indentation and make java.io imports explicit ------------- Changes: - all: https://git.openjdk.org/jdk/pull/29718/files - new: https://git.openjdk.org/jdk/pull/29718/files/9dd1613c..73d9aeba Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=29718&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=29718&range=01-02 Stats: 37 lines in 1 file changed: 7 ins; 0 del; 30 mod Patch: https://git.openjdk.org/jdk/pull/29718.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/29718/head:pull/29718 PR: https://git.openjdk.org/jdk/pull/29718
On Fri, 13 Feb 2026 19:22:01 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Null checks potentially mask a more serious, fundamental problem and should be removed to allow a `NullPointerException` to be throw if the variable is `null`.
Brian Burkhalter has updated the pull request incrementally with one additional commit since the last revision:
8377910: Fix indentation and make java.io imports explicit
Marked as reviewed by alanb (Reviewer). ------------- PR Review: https://git.openjdk.org/jdk/pull/29718#pullrequestreview-3801172808
On Fri, 13 Feb 2026 17:14:31 GMT, Brian Burkhalter <bpb@openjdk.org> wrote:
Null checks potentially mask a more serious, fundamental problem and should be removed to allow a `NullPointerException` to be throw if the variable is `null`.
This pull request has now been integrated. Changeset: 7489f75d Author: Brian Burkhalter <bpb@openjdk.org> URL: https://git.openjdk.org/jdk/commit/7489f75dbdb1358b7f905aad2d1510b7ffc173bf Stats: 44 lines in 1 file changed: 10 ins; 2 del; 32 mod 8377910: Minor cleanup of java/io/FileDescriptor/Sharing.java Reviewed-by: alanb ------------- PR: https://git.openjdk.org/jdk/pull/29718
participants (2)
-
Alan Bateman
-
Brian Burkhalter