Alan thanks for the feedback ...
the implementation is based on what is currently being used in the tests
via the existing
set of NameService implementations.
yes, the processing of the hosts file returns the first mapping it
encounters.
The functionality is to provide similar to that which currently exists
in the various
NameService implementations, such as SimpleNameService.
This would, afaik also, be in line with name service library functions
processing /etc/hosts file.
WRT returning a list of ip addresses, if that is needed then we can
amend the implementation, but
current implementation of tests' NameServices implementations didn't
provide or use such
functionality
Again, none of the tests had a NameService implementation
(SimpleNameService etc,) supporting IPv6, so this determined
what went into the current implementation. It is something that could be
added in future when required.
wrt comments, they could be accommodated, but note that the structure of
a jdk.net.hosts.file is not the same as
/etc/hosts, which maps an IP address to a list of host aliases. If you
that we change the mapping order and the format
we can do that.
wrt illegal_state_exception token, I left this in, thinking that it was
worthwhile as it tests a concurrency issue within InetAddress and the
InetAddress cache.
We can remove this also.
i'll add a transient declaration to re-enforce their absence from
serialized form.
So, do we wish to change the mapping structure, as per /etc/hosts ?
do we wish that a list of ip addresses are returned, even if tests don't
currently need it ?
regards
Mark
On 22/11/2015 09:56, Alan Bateman wrote:
On 12/11/2015 16:46, Mark Sheppard wrote:
Hi,
based on feedback from first review the updates have been amended
please oblige and review the current set of changes as per
http://cr.openjdk.java.net/~msheppar/8134577/webrev.02/
I'm looked through the changes in this webrev.
At a high-level then it's great to see this troublesome internal
provider interface going away. Using the property jdk.net.hosts.file
to specify a hosts file that tests can use seems reasonable.
In a hosts file then a host with several IP addresses can be listed on
several lines. If I read lookupAllHostAddr correctly then it's
ignoring this case and will only find the first line. For testing
purposes then should we allow for hosts file where a host is defined
to have multiple addresses?
Another question is whether IPv6 addresses should be supported, it
looks like lookupAllHostAddr only expects IPv4 addresses.
The other thing is comments (#) ? I'm sure it could be useful to be
able to copy a hosts file and use for testing.
The token illegal_state_exception still looks strange to me as it
means InetAddress.getByName will throw an exception that it is not
specified to throw. I see the test Hang is using it but that test is
for the provider interface that is removed so I assume the test should
be removed too.
A minor point but useLocalHostFileLookpu and hostFilename are static
so I assume the transient modifier isn't needed.
-Alan.
