Hi Aleksei, Latest webrev(01) looks good to me, one minor comment "PREFER_IPV4_STACK" variable naming convention is not consistent with other similar variables(preferIPv6Address) in InetAddress.java.
Thanks, Vyom On Mon, May 25, 2020 at 4:18 PM Aleks Efimov <aleksej.efi...@oracle.com> wrote: > Hi Alan, Daniel, > > Thank you for looking into this change. I've cleaned-up the fix and the > test according to your comments. > Modified fix can be viewed here: > http://cr.openjdk.java.net/~aefimov/8244958/01 > > Kind Regards, > Aleksei > > On 24/05/2020 08:12, Alan Bateman wrote: > > On 22/05/2020 16:45, Aleks Efimov wrote: > >> Hi, > >> > >> The "java.net.preferIPv4Stack" and "java.net.preferIPv6Addresses" > >> system properties do not affect the addresses and their order, > >> returned by the HostsFileNameService provider that can be created by > >> specifying "jdk.net.hosts.file" system property. > >> The following fix analyses the system properties and > >> re-orders/filters the returned IP addresses, if needed. Plus, small > >> clean-up changes: > >> http://cr.openjdk.java.net/~aefimov/8244958/00 > > The approach looks okay but I agree with Daniel on consistent naming > > and cleanup. > > > > Another one to look is arrangeAddresses/appendAddresses as it's > > surprising to mutate inet4Addresses or inet6Addresses with addresses > > of the other type. It would be much cleaner to return a List > > (inetAddress or a new List), then invoke toArray on the result and > > this will avoid calling toArray in 3 places. If you using List in the > > signatories then it will also be a lot cleaned as the caller should > > not need to provide a specific type of List. > > > > Minor nit in the test but PREFER4_SP_VALUE and PREFER6_SP_VALUE can be > > renamed to make the usages in the test easier to read. Also good to > > add "_LIST" to IPV4_THEN_IPV6 to make it clearer at the use sites that > > they are dealing with Lists. I think getExpectedAddressesArray would > > be a bit clear if you assigned the result of the switch expression to > > a variable, then return list.toArray(String::new). > > > > -Alan > > -- Thanks, Vyom
