Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v3]
On Wed, 16 Mar 2022 17:47:15 GMT, Aleksei Efimov wrote: >> Thanks for noticing that Jaikiran! >> >> Both `ipv4_available()` and `ipv6_available()` are defined to return `jint` >> so the implementation in `Java_java_net_InetAddress_isIPv6Supported` is >> arguably the more correct (provided that the method being called returns 0 >> when the corresponding IP version is not available). >> >> The signature in the comment should be changed as you suggest though. >> >> Possibly we should consider having both methods use the same construct... > > Current version should be ok since variables returned by `ipv4_available()` > and `ipv6_available()` can only be initialized to `JNI_FALSE` or `JNI_TRUE`: > - `Java_java_net_InetAddress_isIPv4Available`[InetAddress.c] returns result > of `ipv4_available`[net_util.c] > - `ipv4_available`[net_util.c] returns `IPv4_available` variable value > - `IPv4_available` value is initalized with a result returned by > `IPv4_supported()`[net_util_md.c] > - `IPv4_supported()`[net_util_md.c] can return `JNI_TRUE` or `JNI_FALSE` only > (in both Unix and Windows implementations) > - Same stands true for `ipv6_available()`: only `JNI_TRUE` or `JNI_FALSE` > can be returned Thank you Aleksei for those details and the changes. - PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v3]
On Wed, 16 Mar 2022 17:51:17 GMT, Aleksei Efimov wrote: >> Hi, >> >> This cleanup change removes `InetAddressImplFactory` class from >> `InetAddress`. The list of changes: >> - Remove `InetAddressImplFactory` from `InetAddress` >> - Since `isIPv6Supported` native code is identical for Windows and Unix >> implementations it was moved to the libnet's `InetAddress.c`. >> - `InetAddressImplFactory.c` Windows and Unix implementations were removed >> >> Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change > > Aleksei Efimov has updated the pull request incrementally with one additional > commit since the last revision: > > Update Java_java_net_InetAddress_isIPv4Available signature comment Marked as reviewed by jpai (Committer). - PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8280400: JDK 19 L10n resource files update - msgdrop 10 [v4]
On Wed, 16 Mar 2022 18:31:55 GMT, Alisen Chung wrote: >> msg drop for jdk19, Mar 9, 2022 > > Alisen Chung has updated the pull request incrementally with one additional > commit since the last revision: > > removed incorrect translation of compiler configuration file LGTM. Thanks for the change. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7765
Re: RFR: 8280400: JDK 19 L10n resource files update - msgdrop 10 [v4]
> msg drop for jdk19, Mar 9, 2022 Alisen Chung has updated the pull request incrementally with one additional commit since the last revision: removed incorrect translation of compiler configuration file - Changes: - all: https://git.openjdk.java.net/jdk/pull/7765/files - new: https://git.openjdk.java.net/jdk/pull/7765/files/d5c9b884..715a6ad7 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7765&range=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7765&range=02-03 Stats: 2310 lines in 3 files changed: 0 ins; 2310 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7765.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7765/head:pull/7765 PR: https://git.openjdk.java.net/jdk/pull/7765
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v3]
On Wed, 16 Mar 2022 17:51:17 GMT, Aleksei Efimov wrote: >> Hi, >> >> This cleanup change removes `InetAddressImplFactory` class from >> `InetAddress`. The list of changes: >> - Remove `InetAddressImplFactory` from `InetAddress` >> - Since `isIPv6Supported` native code is identical for Windows and Unix >> implementations it was moved to the libnet's `InetAddress.c`. >> - `InetAddressImplFactory.c` Windows and Unix implementations were removed >> >> Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change > > Aleksei Efimov has updated the pull request incrementally with one additional > commit since the last revision: > > Update Java_java_net_InetAddress_isIPv4Available signature comment Thanks for checking the result returned by ipv6_available() and ipv4_available(). Based on that information I believe what you have is good. - Marked as reviewed by dfuchs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v3]
On Wed, 16 Mar 2022 14:34:49 GMT, Daniel Fuchs wrote: >> I agree that it could be simplified to match >> `Java_java_net_InetAddress_isIPv4Available`. Changed in >> 49fdd576cade2e97639f827f9db6d0f1e31101e2 > > Thanks for noticing that Jaikiran! > > Both `ipv4_available()` and `ipv6_available()` are defined to return `jint` > so the implementation in `Java_java_net_InetAddress_isIPv6Supported` is > arguably the more correct (provided that the method being called returns 0 > when the corresponding IP version is not available). > > The signature in the comment should be changed as you suggest though. > > Possibly we should consider having both methods use the same construct... Current version should be ok since variables returned by `ipv4_available()` and `ipv6_available()` can only be initialized to `JNI_FALSE` or `JNI_TRUE`: - `Java_java_net_InetAddress_isIPv4Available`[InetAddress.c] returns result of `ipv4_available`[net_util.c] - `ipv4_available`[net_util.c] returns `IPv4_available` variable value - `IPv4_available` value is initalized with a result returned by `IPv4_supported()`[net_util_md.c] - `IPv4_supported()`[net_util_md.c] can return `JNI_TRUE` or `JNI_FALSE` only (in both Unix and Windows implementations) - Same stands true for `ipv6_available()`: only `JNI_TRUE` or `JNI_FALSE` can be returned - PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v3]
> Hi, > > This cleanup change removes `InetAddressImplFactory` class from > `InetAddress`. The list of changes: > - Remove `InetAddressImplFactory` from `InetAddress` > - Since `isIPv6Supported` native code is identical for Windows and Unix > implementations it was moved to the libnet's `InetAddress.c`. > - `InetAddressImplFactory.c` Windows and Unix implementations were removed > > Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision: Update Java_java_net_InetAddress_isIPv4Available signature comment - Changes: - all: https://git.openjdk.java.net/jdk/pull/7842/files - new: https://git.openjdk.java.net/jdk/pull/7842/files/49fdd576..0b75bb45 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=01-02 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/7842.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7842/head:pull/7842 PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v2]
On Wed, 16 Mar 2022 14:25:44 GMT, Aleksei Efimov wrote:
>> src/java.base/share/native/libnet/InetAddress.c line 96:
>>
>>> 94: } else {
>>> 95: return JNI_FALSE;
>>> 96: }
>>
>> I don't have knowledge of C or JNI, but the
>> `Java_java_net_InetAddress_isIPv4Available` currently does:
>>
>>
>> return ipv4_available();
>>
>> So maybe a similar construct can be used here instead of if/else blocks? I'm
>> guessing `ipv6_available()` will be returning values that are compatible
>> with `JNI_TRUE/JNI_FALSE`.
>
> I agree that it could be simplified to match
> `Java_java_net_InetAddress_isIPv4Available`. Changed in
> 49fdd576cade2e97639f827f9db6d0f1e31101e2
Thanks for noticing that Jaikiran!
Both `ipv4_available()` and `ipv6_available()` are defined to return `jint` so
the implementation in `Java_java_net_InetAddress_isIPv6Supported` is arguably
the more correct (provided that the method being called returns 0 when the
corresponding IP version is not available).
The signature in the comment should be changed as you suggest though.
Possibly we should consider having both methods use the same construct...
-
PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v2]
> Hi, > > This cleanup change removes `InetAddressImplFactory` class from > `InetAddress`. The list of changes: > - Remove `InetAddressImplFactory` from `InetAddress` > - Since `isIPv6Supported` native code is identical for Windows and Unix > implementations it was moved to the libnet's `InetAddress.c`. > - `InetAddressImplFactory.c` Windows and Unix implementations were removed > > Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change Aleksei Efimov has updated the pull request incrementally with one additional commit since the last revision: Address review comments - Changes: - all: https://git.openjdk.java.net/jdk/pull/7842/files - new: https://git.openjdk.java.net/jdk/pull/7842/files/0af45a2d..49fdd576 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=00-01 Stats: 6 lines in 1 file changed: 0 ins; 4 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7842.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7842/head:pull/7842 PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress [v2]
On Wed, 16 Mar 2022 13:49:39 GMT, Jaikiran Pai wrote:
>> Aleksei Efimov has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Address review comments
>
> src/java.base/share/native/libnet/InetAddress.c line 88:
>
>> 86: * Class: java_net_InetAddress
>> 87: * Method:isIPv6Supported
>> 88: * Signature: ()I
>
> Hello Aleksei, should this be `()Z` to represent the binary name of the
> boolean type
> https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.3.2-200
Hi Jaikiran,
Yes, you're right - it should be `()Z`. The impl copied from
`src/java.base/unix/native/libnet/InetAddressImplFactory.c` had incorrect
header. Fixed in 49fdd576cade2e97639f827f9db6d0f1e31101e2
> src/java.base/share/native/libnet/InetAddress.c line 96:
>
>> 94: } else {
>> 95: return JNI_FALSE;
>> 96: }
>
> I don't have knowledge of C or JNI, but the
> `Java_java_net_InetAddress_isIPv4Available` currently does:
>
>
> return ipv4_available();
>
> So maybe a similar construct can be used here instead of if/else blocks? I'm
> guessing `ipv6_available()` will be returning values that are compatible with
> `JNI_TRUE/JNI_FALSE`.
I agree that it could be simplified to match
`Java_java_net_InetAddress_isIPv4Available`. Changed in
49fdd576cade2e97639f827f9db6d0f1e31101e2
-
PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress
On Wed, 16 Mar 2022 13:26:36 GMT, Aleksei Efimov wrote:
> Hi,
>
> This cleanup change removes `InetAddressImplFactory` class from
> `InetAddress`. The list of changes:
> - Remove `InetAddressImplFactory` from `InetAddress`
> - Since `isIPv6Supported` native code is identical for Windows and Unix
> implementations it was moved to the libnet's `InetAddress.c`.
> - `InetAddressImplFactory.c` Windows and Unix implementations were removed
>
> Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change
src/java.base/share/native/libnet/InetAddress.c line 96:
> 94: } else {
> 95: return JNI_FALSE;
> 96: }
I don't have knowledge of C or JNI, but the
`Java_java_net_InetAddress_isIPv4Available` currently does:
return ipv4_available();
So maybe a similar construct can be used here instead of if/else blocks? I'm
guessing `ipv6_available()` will be returning values that are compatible with
`JNI_TRUE/JNI_FALSE`.
-
PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282917: Remove InetAddressImplFactory from InetAddress
On Wed, 16 Mar 2022 13:26:36 GMT, Aleksei Efimov wrote: > Hi, > > This cleanup change removes `InetAddressImplFactory` class from > `InetAddress`. The list of changes: > - Remove `InetAddressImplFactory` from `InetAddress` > - Since `isIPv6Supported` native code is identical for Windows and Unix > implementations it was moved to the libnet's `InetAddress.c`. > - `InetAddressImplFactory.c` Windows and Unix implementations were removed > > Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change src/java.base/share/native/libnet/InetAddress.c line 88: > 86: * Class: java_net_InetAddress > 87: * Method:isIPv6Supported > 88: * Signature: ()I Hello Aleksei, should this be `()Z` to represent the binary name of the boolean type https://docs.oracle.com/javase/specs/jvms/se17/html/jvms-4.html#jvms-4.3.2-200 - PR: https://git.openjdk.java.net/jdk/pull/7842
RFR: 8282917: Remove InetAddressImplFactory from InetAddress
Hi, This cleanup change removes `InetAddressImplFactory` class from `InetAddress`. The list of changes: - Remove `InetAddressImplFactory` from `InetAddress` - Since `isIPv6Supported` native code is identical for Windows and Unix implementations it was moved to the libnet's `InetAddress.c`. - `InetAddressImplFactory.c` Windows and Unix implementations were removed Testing: jdk-tier1 to jdk-tier3 tests show no failures related to the change - Commit messages: - Merge branch 'master' into JDK-8282917_Remove_InetAddressImplFactory - 8282917: Remove InetAddressImplFactory from InetAddress Changes: https://git.openjdk.java.net/jdk/pull/7842/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=7842&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282917 Stats: 125 lines in 4 files changed: 18 ins; 105 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7842.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7842/head:pull/7842 PR: https://git.openjdk.java.net/jdk/pull/7842
Re: RFR: 8282536: java.net.InetAddress should be a sealed class [v3]
> The following fix seals the `java.net.InetAddress` class and permits only two > implementations - `java.net.Inet4Address` and `java.net.Inet6Address`. > > No issues have been detected by regression and JCK tests. > > Links: [JBS](https://bugs.openjdk.java.net/browse/JDK-8282536) > [CSR](https://bugs.openjdk.java.net/browse/JDK-8282880) Aleksei Efimov 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 JDK-8282536_Seal_InetAddress - Cleanup unnessecary class loader checks, remove readObjectNoData - Add qualified Serializable usage with import - 8282536: java.net.InetAddress should be a sealed class - Changes: - all: https://git.openjdk.java.net/jdk/pull/7789/files - new: https://git.openjdk.java.net/jdk/pull/7789/files/f6901f09..44e6ab12 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=7789&range=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=7789&range=01-02 Stats: 10743 lines in 405 files changed: 6113 ins; 2397 del; 2233 mod Patch: https://git.openjdk.java.net/jdk/pull/7789.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7789/head:pull/7789 PR: https://git.openjdk.java.net/jdk/pull/7789
