Re: RFR: 8263442: Potential bug in jdk.internal.net.http.common.Utils.CONTEXT_RESTRICTED [v2]
> Hi, > > The fix for the reported bug in Utils.CONTEXT_RESTRICTED caused a couple of > regression failures, which turned out to be another bug exposed by this fix > where HTTP/1.1 CONNECT requests with authentication were filtering out proxy > authentication headers wrongly. This was because the HttpRequestImpl created > for the repeated CONNECT was putting the system headers in the user headers > area of the HttpRequestImpl. The fix for that is to supply the user and > system headers direct to the place where the new HttpRequestImpl is created. > > Thanks > Michael Michael McMahon has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision: - Merge branch 'master' into 8263442 - Merge branch 'master' into 8263442 - uncovered an existing bug in first commit. Fixed here - Fix for 8263442 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2977/files - new: https://git.openjdk.java.net/jdk/pull/2977/files/b73dfb73..0bcaa25a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2977&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2977&range=00-01 Stats: 25188 lines in 1348 files changed: 20405 ins; 1802 del; 2981 mod Patch: https://git.openjdk.java.net/jdk/pull/2977.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2977/head:pull/2977 PR: https://git.openjdk.java.net/jdk/pull/2977
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Sat, 13 Mar 2021 22:45:30 GMT, Alex Blewitt wrote: > Sonar displays a warning message that modifiers should be declared in the > order listed in the JLS; specifically, that isntead of using `final static` > the `static final` should be preferred. > > This fixes the issues in the `java.base` package for ease of reviewing. > > https://sonarcloud.io/project/issues?id=shipilev_jdk&languages=java&resolved=false&rules=java%3AS1124 Marked as reviewed by redestad (Reviewer). src/java.base/share/classes/com/sun/security/ntlm/NTLMException.java line 52: > 50: * from server. > 51: */ > 52: //public static final int DOMAIN_UNMATCH = 3; Maybe this one ought to be removed instead? - PR: https://git.openjdk.java.net/jdk/pull/2993
RFR: 8263818: Initial changesRelease JNI local references in get/set-InetXXAddress-member helper functions of net_util.c
As per the Java Native Interface Specification: "All Java objects returned by JNI functions are local references." [1]. The get/set-InetXXAddress-member helper functions in net_util.c retrieve a local reference to the internal `holder` before operating on the InetAddress object. These functions are used in many places by the native code, and should release any local references before returning. [1] https://docs.oracle.com/en/java/javase/16/docs/specs/jni/design.html#global-and-local-references - Commit messages: - Initial changes Changes: https://git.openjdk.java.net/jdk/pull/3074/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=3074&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263818 Stats: 24 lines in 1 file changed: 18 ins; 0 del; 6 mod Patch: https://git.openjdk.java.net/jdk/pull/3074.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/3074/head:pull/3074 PR: https://git.openjdk.java.net/jdk/pull/3074
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Wed, 17 Mar 2021 12:31:22 GMT, Claes Redestad wrote: >> Sonar displays a warning message that modifiers should be declared in the >> order listed in the JLS; specifically, that isntead of using `final static` >> the `static final` should be preferred. >> >> This fixes the issues in the `java.base` package for ease of reviewing. >> >> https://sonarcloud.io/project/issues?id=shipilev_jdk&languages=java&resolved=false&rules=java%3AS1124 > > src/java.base/share/classes/com/sun/security/ntlm/NTLMException.java line 52: > >> 50: * from server. >> 51: */ >> 52: //public static final int DOMAIN_UNMATCH = 3; > > Maybe this one ought to be removed instead? Yeah, I agree. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8187450: JNI local refs exceeds capacity warning in NetworkInterface::getAll [v4]
On Tue, 16 Mar 2021 15:41:21 GMT, Andrew Haley wrote: >> Thanks for making these changes. This is much easier to review now. I'm not >> an expert with JNI ref - but the logic now looks right. > > Please hold off on this; there's an issue to clear up. @theRealAph Please let me know when this PR can be integrated. ( it has been reviewed, and tier 1-4 tests run fine ) - PR: https://git.openjdk.java.net/jdk/pull/2963
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Thu, 18 Mar 2021 15:08:09 GMT, Aleksey Shipilev wrote: >> src/java.base/share/classes/com/sun/security/ntlm/NTLMException.java line 52: >> >>> 50: * from server. >>> 51: */ >>> 52: //public static final int DOMAIN_UNMATCH = 3; >> >> Maybe this one ought to be removed instead? > > Yeah, I agree. Is that there to indicate a placeholder value that was once used and is kept for documentation purposes? Should the corresponding JavaDoc be removed as well? Should I do this in the same commit/PR as this one, or submit a new PR? Would prefer to avoid conflating fixes if possible so that if one needs to be reverted we don't revert the related changes. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Thu, 18 Mar 2021 16:42:39 GMT, Alex Blewitt wrote: >> Yeah, I agree. > > Is that there to indicate a placeholder value that was once used and is kept > for documentation purposes? Should the corresponding JavaDoc be removed as > well? Should I do this in the same commit/PR as this one, or submit a new PR? > Would prefer to avoid conflating fixes if possible so that if one needs to be > reverted we don't revert the related changes. There's another constant with value 3 defined, so I think this is just some left-over. If you prefer separating out the removal to another RFE I'd remove this particular change from this PR. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base
On Thu, 18 Mar 2021 14:50:43 GMT, Claes Redestad wrote: >> Sonar displays a warning message that modifiers should be declared in the >> order listed in the JLS; specifically, that isntead of using `final static` >> the `static final` should be preferred. >> >> This fixes the issues in the `java.base` package for ease of reviewing. >> >> https://sonarcloud.io/project/issues?id=shipilev_jdk&languages=java&resolved=false&rules=java%3AS1124 > > Marked as reviewed by redestad (Reviewer). If I have other fixes for different modules, should I file PRs with the same bug number e.g. "8263658: Use the blessed modifier order in java.logging/java.desktop" or should we have separate bug numbers for them? - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
> Sonar displays a warning message that modifiers should be declared in the > order listed in the JLS; specifically, that isntead of using `final static` > the `static final` should be preferred. > > This fixes the issues in the `java.base` package for ease of reviewing. > > https://sonarcloud.io/project/issues?id=shipilev_jdk&languages=java&resolved=false&rules=java%3AS1124 Alex Blewitt has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains one new commit since the last revision: 8263658: Use the blessed modifier order in java.base - Changes: - all: https://git.openjdk.java.net/jdk/pull/2993/files - new: https://git.openjdk.java.net/jdk/pull/2993/files/470f7066..86aa9a34 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=2993&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=2993&range=00-01 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2993.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2993/head:pull/2993 PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
On Thu, 18 Mar 2021 16:50:39 GMT, Claes Redestad wrote: >> Is that there to indicate a placeholder value that was once used and is kept >> for documentation purposes? Should the corresponding JavaDoc be removed as >> well? Should I do this in the same commit/PR as this one, or submit a new >> PR? Would prefer to avoid conflating fixes if possible so that if one needs >> to be reverted we don't revert the related changes. > > There's another constant with value 3 defined, so I think this is just some > left-over. > > If you prefer separating out the removal to another RFE I'd remove this > particular change from this PR. Filed #3076 for the removal and updated this PR without it - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
On Thu, 18 Mar 2021 16:51:35 GMT, Alex Blewitt wrote: > If I have other fixes for different modules, should I file PRs with the same > bug number e.g. "8263658: Use the blessed modifier order in > java.logging/java.desktop" or should we have separate bug numbers for them? Separate bug numbers is necessary. Unless you repurpose / widen this PR to include more modules. A word of advice: Avoid git rebase + force push after opening a PR for review, since it might mess up the review context. Preferably either merge in changes from main, or just keep adding commits without syncing up - the system will ensure your patch can be merged in without conflicts. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
On Thu, 18 Mar 2021 17:03:28 GMT, Claes Redestad wrote: >> If I have other fixes for different modules, should I file PRs with the same >> bug number e.g. "8263658: Use the blessed modifier order in >> java.logging/java.desktop" or should we have separate bug numbers for them? > >> If I have other fixes for different modules, should I file PRs with the same >> bug number e.g. "8263658: Use the blessed modifier order in >> java.logging/java.desktop" or should we have separate bug numbers for them? > > Separate bug numbers is necessary. Unless you repurpose / widen this PR to > include more modules. > > A word of advice: Avoid git rebase + force push after opening a PR for > review, since it might mess up the review context. Preferably either merge in > changes from main, or just keep adding commits without syncing up - the > system will ensure your patch can be merged in without conflicts. I'm happy to either widen the scope of this PR or to submit multiple but I have to rely on the kindness of strangers to create appropriate bugs in the system. On the one hand I don't want to cause a lot of noise but on the other having smaller independent commits (particularly if they hit a lot of files/modules) seems like a more sensible plan to me. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
On Thu, 18 Mar 2021 17:06:04 GMT, Alex Blewitt wrote: >>> If I have other fixes for different modules, should I file PRs with the >>> same bug number e.g. "8263658: Use the blessed modifier order in >>> java.logging/java.desktop" or should we have separate bug numbers for them? >> >> Separate bug numbers is necessary. Unless you repurpose / widen this PR to >> include more modules. >> >> A word of advice: Avoid git rebase + force push after opening a PR for >> review, since it might mess up the review context. Preferably either merge >> in changes from main, or just keep adding commits without syncing up - the >> system will ensure your patch can be merged in without conflicts. > > I'm happy to either widen the scope of this PR or to submit multiple but I > have to rely on the kindness of strangers to create appropriate bugs in the > system. On the one hand I don't want to cause a lot of noise but on the > other having smaller independent commits (particularly if they hit a lot of > files/modules) seems like a more sensible plan to me. There's some extra churn in splitting it up, sure, but different modules are often maintained by different people so getting changes that span the entire JDK reviewed might actually take you longer. YMMV. I can assist creating RFEs. - PR: https://git.openjdk.java.net/jdk/pull/2993
Re: RFR: 8263658: Use the blessed modifier order in java.base [v2]
On Thu, 18 Mar 2021 17:02:57 GMT, Alex Blewitt wrote: >> Sonar displays a warning message that modifiers should be declared in the >> order listed in the JLS; specifically, that isntead of using `final static` >> the `static final` should be preferred. >> >> This fixes the issues in the `java.base` package for ease of reviewing. >> >> https://sonarcloud.io/project/issues?id=shipilev_jdk&languages=java&resolved=false&rules=java%3AS1124 > > Alex Blewitt has refreshed the contents of this pull request, and previous > commits have been removed. The incremental views will show differences > compared to the previous content of the PR. Marked as reviewed by redestad (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2993
