On Mon, 18 Sep 2023 23:44:22 GMT, Chris Plummer <cjplum...@openjdk.org> wrote:
> Is the code starting at lines 748 and 970 still correct after your changes?
I think so, but scrutiny there is very welcome. I expected additional changes
to be necessary.
* For 748, it's handling a situation where IPv4 isn't specifically preferred.
The implementation comment notes 'if preferredAddressFamily is AF_INET6 or not
set'.
* For 970 the first loop won't match, and on the second loop it will try in the
order provided by `getaddrinfo`, which is what we want.
If that sounds right, suggestions for making it clearer / more explicit are
welcome.
> src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c line 1312:
>
>> 1310: * - AF_INET6 if the property is "true";
>> 1311: * - AF_INET if the property is "false".
>> 1312: * - 0 if the property is "false".
>
> I think you meant "system", not "false".
Thanks, updated to `AF_UNSPEC if the property is "system"`
> src/jdk.jdwp.agent/share/native/libdt_socket/socketTransport.c line 1338:
>
>> 1336: preferredAddressFamily = AF_INET;
>> 1337: } else if (strcmp(theValue, "system") == 0) {
>> 1338: preferredAddressFamily = 0;
>
> Would `AF_UNSPEC` (which is 0) be a better choice than a literal 0 here?
SGTM, thanks!
-------------
PR Comment: https://git.openjdk.org/jdk/pull/15796#issuecomment-1724637144
PR Review Comment: https://git.openjdk.org/jdk/pull/15796#discussion_r1329401761
PR Review Comment: https://git.openjdk.org/jdk/pull/15796#discussion_r1329401756
