Hi Vyom,
Thanks for reviewing it. I'm ok with stepping aside of general naming
convention to maintain the compatibility with the naming in InetAddress
code:
InetAddress.HostsFileNameService.PREFER_IPV4_STACK renamed to
InetAddress.HostsFileNameService.preferIPv4Stack
Webrev was updated at the same location:
http://cr.openjdk.java.net/~aefimov/8244958/01
Best,
Aleksei
On 25/05/2020 13:57, Vyom Tiwari wrote:
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 <mailto: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
