Re: RFR: 8333714: Cleanup the usages of CHECK_EXCEPTION_NULL_FAIL macro in java launcher [v2]
On Thu, 6 Jun 2024 13:28:55 GMT, Jaikiran Pai wrote: >> Can I please get a review for this change which proposes to remove the >> `CHECK_EXCEPTION_NULL_FAIL` macro from the `java` launcher code? >> >> This addresses https://bugs.openjdk.org/browse/JDK-8333714. As noted in that >> JBS issue, in a recent PR discussion, it was suggested >> https://github.com/openjdk/jdk/pull/18786#issuecomment-2147452633 that this >> macro should be removed and the failure of a JNI specified operation (the >> ones for which this macro is being used) should be determined based on a >> `NULL` value returned from that function. The commit in this PR removes this >> macros and updates the call sites to do a `NULL` check. >> >> Given the nature of this change, no new tests have been added. tier1, tier2 >> and tier3 testing passed successfully with these changes. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > improve comments Thank you for the review Alan. I'll integrate this in a few hours. - PR Comment: https://git.openjdk.org/jdk/pull/19576#issuecomment-2167053874
Re: RFR: 8333714: Cleanup the usages of CHECK_EXCEPTION_NULL_FAIL macro in java launcher [v2]
On Thu, 6 Jun 2024 13:28:55 GMT, Jaikiran Pai wrote: >> Can I please get a review for this change which proposes to remove the >> `CHECK_EXCEPTION_NULL_FAIL` macro from the `java` launcher code? >> >> This addresses https://bugs.openjdk.org/browse/JDK-8333714. As noted in that >> JBS issue, in a recent PR discussion, it was suggested >> https://github.com/openjdk/jdk/pull/18786#issuecomment-2147452633 that this >> macro should be removed and the failure of a JNI specified operation (the >> ones for which this macro is being used) should be determined based on a >> `NULL` value returned from that function. The commit in this PR removes this >> macros and updates the call sites to do a `NULL` check. >> >> Given the nature of this change, no new tests have been added. tier1, tier2 >> and tier3 testing passed successfully with these changes. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > improve comments This looks okay, it makes these invokeXXX functions easier to read and avoids any confusion on return NULL vs. pending exception. - Marked as reviewed by alanb (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/19576#pullrequestreview-2116198827
Re: RFR: 8333714: Cleanup the usages of CHECK_EXCEPTION_NULL_FAIL macro in java launcher [v2]
On Thu, 6 Jun 2024 13:28:55 GMT, Jaikiran Pai wrote: >> Can I please get a review for this change which proposes to remove the >> `CHECK_EXCEPTION_NULL_FAIL` macro from the `java` launcher code? >> >> This addresses https://bugs.openjdk.org/browse/JDK-8333714. As noted in that >> JBS issue, in a recent PR discussion, it was suggested >> https://github.com/openjdk/jdk/pull/18786#issuecomment-2147452633 that this >> macro should be removed and the failure of a JNI specified operation (the >> ones for which this macro is being used) should be determined based on a >> `NULL` value returned from that function. The commit in this PR removes this >> macros and updates the call sites to do a `NULL` check. >> >> Given the nature of this change, no new tests have been added. tier1, tier2 >> and tier3 testing passed successfully with these changes. > > Jaikiran Pai has updated the pull request incrementally with one additional > commit since the last revision: > > improve comments Any reviews for this please? - PR Comment: https://git.openjdk.org/jdk/pull/19576#issuecomment-2165803780
Re: RFR: 8333714: Cleanup the usages of CHECK_EXCEPTION_NULL_FAIL macro in java launcher [v2]
> Can I please get a review for this change which proposes to remove the > `CHECK_EXCEPTION_NULL_FAIL` macro from the `java` launcher code? > > This addresses https://bugs.openjdk.org/browse/JDK-8333714. As noted in that > JBS issue, in a recent PR discussion, it was suggested > https://github.com/openjdk/jdk/pull/18786#issuecomment-2147452633 that this > macro should be removed and the failure of a JNI specified operation (the > ones for which this macro is being used) should be determined based on a > `NULL` value returned from that function. The commit in this PR removes this > macros and updates the call sites to do a `NULL` check. > > Given the nature of this change, no new tests have been added. tier1, tier2 > and tier3 testing passed successfully with these changes. Jaikiran Pai has updated the pull request incrementally with one additional commit since the last revision: improve comments - Changes: - all: https://git.openjdk.org/jdk/pull/19576/files - new: https://git.openjdk.org/jdk/pull/19576/files/1c2a3dfb..1b3d630a Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=19576=01 - incr: https://webrevs.openjdk.org/?repo=jdk=19576=00-01 Stats: 20 lines in 1 file changed: 0 ins; 8 del; 12 mod Patch: https://git.openjdk.org/jdk/pull/19576.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19576/head:pull/19576 PR: https://git.openjdk.org/jdk/pull/19576
RFR: 8333714: Cleanup the usages of CHECK_EXCEPTION_NULL_FAIL macro in java launcher
Can I please get a review for this change which proposes to remove the `CHECK_EXCEPTION_NULL_FAIL` macro from the `java` launcher code? This addresses https://bugs.openjdk.org/browse/JDK-8333714. As noted in that JBS issue, in a recent PR discussion, it was suggested https://github.com/openjdk/jdk/pull/18786#issuecomment-2147452633 that this macro should be removed and the failure of a JNI specified operation (the ones for which this macro is being used) should be determined based on a `NULL` value returned from that function. The commit in this PR removes this macros and updates the call sites to do a `NULL` check. Given the nature of this change, no new tests have been added. tier1, tier2 and tier3 testing passed successfully with these changes. - Commit messages: - simplify function comments - 8333714: Cleanup the usages of CHECK_EXCEPTION_NULL_FAIL macro in java launcher Changes: https://git.openjdk.org/jdk/pull/19576/files Webrev: https://webrevs.openjdk.org/?repo=jdk=19576=00 Issue: https://bugs.openjdk.org/browse/JDK-8333714 Stats: 76 lines in 1 file changed: 45 ins; 9 del; 22 mod Patch: https://git.openjdk.org/jdk/pull/19576.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19576/head:pull/19576 PR: https://git.openjdk.org/jdk/pull/19576
