RFR: 8302659: Modernize Windows native code for NetworkInterface [v3]

Rich DiCroce duke at openjdk.org
Wed Feb 22 17:40:42 UTC 2023


On Thu, 16 Feb 2023 15:48:17 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:

>> Rich DiCroce has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Forgot to add file
>>  - Resolve review comments
>
> src/java.base/windows/native/libnet/NetworkInterface.c line 2:
> 
>> 1: /*
>> 2:  * Copyright (c) 2003, 2022, Oracle and/or its affiliates. All rights reserved.
> 
> Suggestion:
> 
>  * Copyright (c) 2000, 2023, Oracle and/or its affiliates. All rights reserved.

Fixed.

> src/java.base/windows/native/libnet/NetworkInterface.c line 62:
> 
>> 60: typedef struct _netaddr {
>> 61:     SOCKADDR_INET Address;
>> 62:     SCOPE_ID ScopeId;
> 
> Not used

Removed. Good catch.

> src/java.base/windows/native/libnet/NetworkInterface.c line 67:
> 
>> 65: } netaddr;
>> 66: 
>> 67: BOOL getAddressTables(MIB_UNICASTIPADDRESS_TABLE **uniAddrs, MIB_ANYCASTIPADDRESS_TABLE **anyAddrs) {
> 
> Please mark all functions that are local to this file as `static`, this will help the compiler

Done.

> src/java.base/windows/native/libnet/NetworkInterface.c line 69:
> 
>> 67: BOOL getAddressTables(MIB_UNICASTIPADDRESS_TABLE **uniAddrs, MIB_ANYCASTIPADDRESS_TABLE **anyAddrs) {
>> 68:     ADDRESS_FAMILY addrFamily = ipv6_available() ? AF_UNSPEC : AF_INET;
>> 69:     if (GetUnicastIpAddressTable(addrFamily, uniAddrs) != NO_ERROR) {
> 
> Please add error handling to all WinAPI calls; something like:
> 
> NET_ThrowByNameWithLastError(env, JNU_JAVANETPKG "SocketException", "GetUnicastIpAddressTable");
> 
> You may also need to use `SetLastError(ret)` before calling NET_Throw...; the WinAPI functions are not documented to do it, and we do not have a function that would take the error code as a parameter.

Done.

> src/java.base/windows/native/libnet/NetworkInterface.c line 393:
> 
>> 391:  * Signature: ()[Ljava/net/NetworkInterface;
>> 392:  */
>> 393: JNIEXPORT jobjectArray JNICALL Java_java_net_NetworkInterface_getAll(JNIEnv *env, jclass cls) {
> 
> This may have also fixed [JDK-6798979](https://bugs.openjdk.org/browse/JDK-6798979) and [JDK-8165665](https://bugs.openjdk.org/browse/JDK-8165665) (which is a good thing!)

I agree, it looks like both of those would be fixed by these changes.

-------------

PR: https://git.openjdk.org/jdk/pull/12593



More information about the build-dev mailing list