Vyom,

On 13/05/16 08:23, vyom wrote:
Hi,

Please find the updated
webrev(http://cr.openjdk.java.net/~vtewari/8016521/webrev0.1/index.html
<http://cr.openjdk.java.net/%7Evtewari/8016521/webrev0.1/index.html>), i
incorporated the review comments.

This addresses my comments regarding the code. I'm still not
sure what, if anything, we can do in the area of testing, but
maybe that could be done as a follow up.

One final comment, that I missed earlier. I think
preferIPv6Address can be made final, if you remove the '-1'
initializer.

-Chris.

Thanks,
Vyom


On Thursday 12 May 2016 09:57 PM, Chris Hegarty wrote:
On 10 May 2016, at 20:52, e...@zusammenkunft.net wrote:

Hello,

Love it.
Yes, this is along the same lines as I was thinking.

Not sure about two things, first of all if there are more test cases
(especially assertions) needed and
Right. Maybe a new test that, 1) runs in all three modes, and 2) asserts
that the order of addressed returned from several different InetAddress
getXXX calls, is consistent with what we expect. Though, I’m not sure how
to check ‘system’ other than it must contain the same set of addresses
as 'false' or ‘true’.

I wonder if being able to successfully create a
java.io.channels.DatagramChannel.open(StandardProtocolFamily.INET6)
is a reliable way to tell if IPv6 is supported.

secondly how to handle the prefer=System case for anyAddr and local.
Currently it seems to prefer v4 (since this is the current default)
howver i would expect in the system case to either detect the
prefered AF or use ipv6 (as of rfc). Returning 127.0.1/0.0.0.0 might
actually make the system based address detection unuseable in v6
scenarios.
I agree. Specifically you talking about Inet6AddressImpl
anyLocalAddress() and loopbackAddress(), right?
I think the changes in Inet6AddressImpl need to check:

    if (InetAddress.preferIPv6Address == PREFER_IPV6_VALUE ||
        InetAddress.preferIPv6Address == PREFER_SYSTEM_VALUE)

InetAddressImplFactory already checks for ‘isIPv6Supported’ when
determining which impl to create. If an Inet6AddressImpl is created,
then IPv6 is supported on the system, therefore either
PREFER_SYSTEM_VALUE or PREFER_IPV6_VALUE should
cause anyLocalAddress() and loopbackAddress() to return an
Inet6Address.


Other minor comments:
   Inet6AddressImpl.java: can you statically import the int values
   unix Inet6AddressImpl.c. missing space, ‘if(…)’ L 445

-Chris.

Gruss
Bernd
--
http://bernd.eckenfels.net

-----Original Message-----
From: vyom <vyom.tew...@oracle.com>
To: net-dev <net-dev@openjdk.java.net>
Sent: Di., 10 Mai 2016 12:36
Subject: RFR 8016521: InetAddress should not always re-order
addresses returned from name service

Hi All,

Please review the code changes for the below issue.

Bug       : JDK-8016521 : InetAddress should not always re-order
addresses returned from name service
Webrev :
http://cr.openjdk.java.net/~vtewari/8016521/webrev0.0/index.html
<http://cr.openjdk.java.net/%7Evtewari/8016521/webrev0.0/index.html>

Thanks,
Vyom


Reply via email to