RFR (S): JDK-8250630 JdwpListenTest.java fails on Alpine Linux
Alex Menkov
alexey.menkov at oracle.com
Fri Aug 28 20:10:25 UTC 2020
Hi Dmitry,
Looks good to me.
2 minor nits (no new webrev required):
- copyright year
- indentation in lines 763 and 766
--alex
On 08/27/2020 08:03, Dmitry Samersoff wrote:
> Hello Everybody,
>
> http://cr.openjdk.java.net/~dsamersoff/JDK-8250630/webrev.02/
>
> Webrev is updated, all comments accepted and addressed.
>
> Except:
> > The error code from the inet_pton is not checked.
>
> inet_pton performs conversion of the constant value in our case and the
> only possible reason for it to fail is that the system doesn't support
> IPv6 at all.
>
> -Dmitry
>
>
> On 18.08.2020 3:20, serguei.spitsyn at oracle.com wrote:
>> Hi Dmitry,
>>
>> I agree with Alex, it is better to rename compareIPv6Addr to
>> isEqualIPv6Addr.
>>
>> 705 static int compareIPv6Addr(struct addrinfo *ai, struct in6_addr
>> in6Addr)
>> 706 {
>> 707
>> 708 if (ai->ai_addr->sa_family == AF_INET6) {
>> 709 const struct sockaddr_in6 sa = *((struct sockaddr_in6*) ai->ai_addr);
>> 710 return (memcmp(&sa.sin6_addr, &in6Addr, sizeof(in6Addr)) == 0);
>> 711 }
>> 712
>> 713 return 0;
>> 714 }
>>
>> I think, the lines 707 and 712 are not needed.
>>
>> 725 struct in6_addr mappedAny; ... 761 inet_pton(AF_INET6,
>> "::ffff:0.0.0.0", &mappedAny);
>>
>> The error code from the inet_pton is not checked.
>> Also, it can be useful to pre-initialize the mappedAny.
>>
>> 737 // Try to find bind address of preferred address familty first A
>> dot at the end of comment is missed.
>>
>> 745 if (listenAddr == NULL) {
>> 746 // No address of preferred addres family found, grab the first one
>> 747 listenAddr = &(addrInfo[0]);
>> 748 }
>>
>> The indent has to be 4, not 3.
>> The () brackets are not actually needed but I do not object if you
>> keep them.
>> A dot at the end of comment is missed.
>>
>> 755 // Binding to IN6ADDR_ANY allow us to serve both IPv4 and IPv6
>> connections,
>> 756 // but binding to mapped INADDR_ANY (::ffff:0.0.0.0) allow us to
>> serve IPv4
>> 757 // connections only. So make sure, that IN6ADDR_ANY is preferred over
>> 758 // mapped INADDR_ANY if preferredAddressFamily is AF_INET6 or not set
>>
>> I'd suggest to replace "allow us" => "allows" in two places:
>> "IN6ADDR_ANY allow us" and "(::ffff:0.0.0.0) allow us".
>> Also, it'd better to replace: "So make sure,that" => "Make sure that".
>> A dot at the end of comment is missed.
>>
>> I don't know the network protocols well enough to comment on
>>
>>
>> Thanks,
>> Serguei
>>
>>
>> On 8/17/20 00:21, Dmitry Samersoff wrote:
>>> Hello Everybody,
>>>
>>> Please review the fix:
>>>
>>> https://cr.openjdk.java.net/~dsamersoff/JDK-8250630/webrev.01/
>>>
>>> Binding to IN6ADDR_ANY allow us to serve both IPv4 and IPv6
>>> connections, but binding to mapped INADDR_ANY (::ffff:0.0.0.0) allow
>>> us to serve IPv4 connections only.
>>>
>>> So make sure, that IN6ADDR_ANY is preferred over mapped INADDR_ANY if
>>> preferredAddressFamily is not AF_INET
>>>
>>>
>>> -Dmitry\S
>>>
>>
>
More information about the serviceability-dev
mailing list