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

[Daniel Jeliński]

On Wed, 22 Feb 2023 17:40:31 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 two additional 
> commits since the last revision:
> 
>  - Forgot to add file
>  - Resolve review comments

make/modules/java.base/Lib.gmk line 50:

> 48:     DISABLED_WARNINGS_clang_net_util_md.c := format-nonliteral, \
> 49:     DISABLED_WARNINGS_microsoft_InetAddress.c := 4244, \
> 50:     DISABLED_WARNINGS_microsoft_NetworkInterface.c := 4133, \

This line is no longer necessary

src/java.base/windows/native/libnet/NetworkInterface.c line 110:

> 108:     // set the NetworkInterface's name
> 109:     apiRetVal = ConvertInterfaceLuidToNameW(
> 110:             &(ifRow->InterfaceLuid), (PWSTR) &ifName, 
> NDIS_IF_MAX_BUFFER_SIZE);

Suggestion:

            &(ifRow->InterfaceLuid), ifName, NDIS_IF_MAX_BUFFER_SIZE);

src/java.base/windows/native/libnet/NetworkInterface.c line 118:

> 116:     }
> 117:     name = (*env)->NewString(
> 118:             env, (const jchar *) &ifName, (jsize) wcslen((const wchar_t 
> *) &ifName));

Suggestion:

            env, ifName, (jsize) wcslen(ifName));

src/java.base/windows/native/libnet/NetworkInterface.c line 128:

> 126:     displayName = (*env)->NewString(
> 127:             env, (const jchar *) ifRow->Description,
> 128:             (jsize) wcslen((const wchar_t *) &(ifRow->Description)));

Suggestion:

            env, ifRow->Description,
            (jsize) wcslen(ifRow->Description));

src/java.base/windows/native/libnet/NetworkInterface.c line 295:

> 293:     apiRetVal = GetIfEntry2(ifRow);
> 294:     if (apiRetVal != NO_ERROR) {
> 295:         if (throwIfNotFound && apiRetVal == ERROR_FILE_NOT_FOUND) {

Suggestion:

        if (throwIfNotFound || apiRetVal != ERROR_FILE_NOT_FOUND) {

src/java.base/windows/native/libnet/NetworkInterface.c line 334:

> 332: 
> 333:     ifRow.InterfaceIndex = index;
> 334:     return createNetworkInterfaceForSingleRow(env, FALSE, &ifRow);

once you fix the `throwIfNotFound` issue above, you'll also need special 
handling for index 0 here; `GetIfEntry2` will return ERROR_INVALID_PARAMETER 
when index is zero (zero means not set)

src/java.base/windows/native/libnet/NetworkInterface.c line 360:

> 358:         return NULL;
> 359:     }
> 360:     return createNetworkInterfaceForSingleRow(env, TRUE, &ifRow);

Suggestion:

    return createNetworkInterfaceForSingleRow(env, FALSE, &ifRow);

`ConvertInterfaceNameToLuidW` does not verify interface existence; without this 
change `NetworkInterface.getByName("loopback_1")` would throw instead of 
returning null

src/java.base/windows/native/libnet/NetworkInterface.c line 387:

> 385:             ifRow.InterfaceLuid = uniAddrs->Table[i].InterfaceLuid;
> 386:             result = createNetworkInterfaceForSingleRowWithTables(
> 387:                     env, TRUE, &ifRow, uniAddrs, anyAddrs);

Suggestion:

                    env, FALSE, &ifRow, uniAddrs, anyAddrs);

network interfaces are notoriously disappearing at the least convenient time

src/java.base/windows/native/libnet/NetworkInterface.c line 396:

> 394:             ifRow.InterfaceLuid = anyAddrs->Table[i].InterfaceLuid;
> 395:             result = createNetworkInterfaceForSingleRowWithTables(
> 396:                     env, TRUE, &ifRow, uniAddrs, anyAddrs);

Suggestion:

                    env, FALSE, &ifRow, uniAddrs, anyAddrs);

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

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