On 17 May 2016, at 14:36, vyom <vyom.tew...@oracle.com> wrote: > Hi Chris, > > Please find the updated > webrev(http://cr.openjdk.java.net/~vtewari/8016521/webrev0.3/index.html > <http://cr.openjdk.java.net/%7Evtewari/8016521/webrev0.3/index.html>). i have > did the changes as you suggested.
Thanks. The updated webrev look good. -Chris. > Thanks, > Vyom > > > On Tuesday 17 May 2016 05:04 PM, Chris Hegarty wrote: >> On 17 May 2016, at 10:48, vyom <vyom.tew...@oracle.com> wrote: >> >>> Hi Chris, >>> >>> Please find the updated >>> webrev(http://cr.openjdk.java.net/~vtewari/8016521/webrev0.2/index.html >>> <http://cr.openjdk.java.net/%7Evtewari/8016521/webrev0.2/index.html>). >> This looks good Vyom. Thanks. >> >> One additional change is needed. Apologies, I forgot that the specification >> of java.net.preferIPv6Addresses was in the source tree and not in the >> guides. We will need to update this, something similar to... >> >> diff --git >> a/src/java.base/share/classes/java/net/doc-files/net-properties.html >> b/src/java.base/share/classes/java/net/doc-files/net-properties.html >> --- a/src/java.base/share/classes/java/net/doc-files/net-properties.html >> +++ b/src/java.base/share/classes/java/net/doc-files/net-properties.html >> @@ -58,7 +58,8 @@ >> applications that depend on the representation of an IPv4 address >> (e.g. 192.168.1.1). This property can be set to <B>true</B> to >> change that preference and use IPv6 addresses over IPv4 ones where >> - possible.</P> >> + possible, or <B>system</B> to preserve the order of the addresses as >> + returned by the operating system.</P> >> </UL> >> <P>Both of these properties are checked only once, at startup.</P> >> <a name="Proxies"></a> >> >> If you agree, can you fold it into your patch, and generate a changeset. >> >> -Chris. >> >>> Thanks, >>> Vyom >>> >>> >>> On Friday 13 May 2016 06:33 PM, Chris Hegarty wrote: >>>> 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 >>>>>>> >