On Thu, 16 Feb 2023 15:48:17 GMT, Daniel Jeliński <[email protected]> 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
