On Thu, 16 Feb 2023 16:49:09 GMT, Rich DiCroce <d...@openjdk.org> wrote:
>> Improves performance and correctness, as discussed on the net-dev mailing
>> list.
>
> Rich DiCroce has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Limit line length to 80-ish characters
Overall the changes look good. A few comments below.
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.
src/java.base/windows/native/libnet/NetworkInterface.c line 62:
> 60: typedef struct _netaddr {
> 61: SOCKADDR_INET Address;
> 62: SCOPE_ID ScopeId;
Not used
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
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.
src/java.base/windows/native/libnet/NetworkInterface.c line 72:
> 70: return FALSE;
> 71: }
> 72: if (GetAnycastIpAddressTable(addrFamily, anyAddrs) != NO_ERROR) {
Were the anycast addresses covered by the existing code, or is this a
functional change?
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!)
src/java.base/windows/native/libnet/NetworkInterface.c line 523:
> 521: * Signature: (Ljava/lang/String;I)Z
> 522: */
> 523: JNIEXPORT jboolean JNICALL
> Java_java_net_NetworkInterface_supportsMulticast0(JNIEnv *env, jclass cls,
> jstring name, jint index) {
I'm not a big fan of using WBEM here. Also, I'm still trying to figure out when
(if ever) this method is supposed to return false
-------------
PR: https://git.openjdk.org/jdk/pull/12593
