Rob,
See below -
ll. 210 have to be fixed, other comments is just an opinion.
Inet4AddressImpl.c:
ll.132 - it might be better to move initialization to a separate
function, the same way as in Inet6 - I really like this idea.
Inet6AddressImpl.c
ll. 126.
it's better to move static int initialized into initializeInetClasses()
and don't check it ll. 282.
ll. 170
it looks like rest of the code uses NI_MAXHOST also we have to check
results of JVM_GetHostName if it returns -1 it's probably better to
bailout immediately.
ll. 193 and below - wrong indent
4)
ll. 210
we can have more than one v4 address so
if (addrs4 >= 1)
have to be here.
*.
Your include ipv6 loopback in the list if interface has no ipv4
addresses, I'm not sure this logic is correct. On my opinion it's better
to never include ipv6 loopbacks.
*.
Is it better to rename:
includeLoopback => includeLoopbacks
ll. 218
It might be better to calculate arraySize under if at ll. 210 to make
code better readable
ll. 236
It might be better to split if statement to make it more readable at the
cost of duplicating iter = iter->ifa_next; line.
e.g.
while (iter != NULL) {
...
if (family != AF_INET6 and family != AF_INET) {
iter = iter->ifa_next;
continue;
}
if ((!includeV6 and family == AF_INET6)) {
iter = iter->ifa_next;
continue;
}
if (!includeLoopback and isLoopback) {
iter = iter->ifa_next;
continue;
}
if (iter->ifa_name[0] != '\0' && iter->ifa_addr) {
// Copy address to Java array
....
iter = iter->ifa_next;
continue; // redundant just for readability
}
}
ll.244
I'm not sure it's good to return partially complete array in case of
object allocation fail. Probably you should throw
JNU_ThrowOutOfMemoryError(env, "Object allocation failed");
-Dmitry
On 2013-09-20 18:58, Rob McKenna wrote:
> After a brief discussion with Chris, we decided to revert the position
> of the call to lookupIfLocalAddrs so as to give the local host primacy
> over DNS.
>
> Latest (and hopefully last) webrev here:
>
> http://cr.openjdk.java.net/~robm/7180557/webrev.03/
>
> -Rob
>
> On 14/09/13 00:00, Rob McKenna wrote:
>> Hi Bernd,
>>
>> I should have said in the context of this bug. What I meant was
>> removing AI_CANONNAME doesn't resolve the issue as far as Mac is
>> concerned. I.e. I still see the UnknownHostException. In this
>> particular case the hostname is not set via the hosts file.
>>
>> In the latest webrev the call to getifaddrs only occurs if getaddrinfo
>> fails.
>>
>> -Rob
>>
>> On 13/09/13 20:28, Bernd Eckenfels wrote:
>>> Am 13.09.2013, 19:32 Uhr, schrieb Rob McKenna <rob.mcke...@oracle.com>:
>>>> W.r.t. the use of AI_CANONNAME, this doesn't actually make a
>>>> difference in the context of this fix, but is definitely something
>>>> that should be looked at. I'll put it on the todo list.
>>>
>>> I think it does make a difference: If you remove the CANON flag
>>> getaddrinfo() will not do DNS lookups when the host is configured to
>>> prefer the hosts file (which it should do on Linux and OSX). And so
>>> the platform specific lookupIfLocalhost() can be put after the
>>> getaddrinfo() (again).
>>>
>>> I actually think the bug "exists" on all platforms. If getaddrinfo()
>>> fails because neighter hosts nor DNS file finds the name this can
>>> happen on all platforms. I dont think it helps to add a fallback only
>>> on MACOSX (and there is certainly no need to prefer the fallback then).
>>>
>>> Gruss
>>> Bernd
>>
>
--
Dmitry Samersoff
Oracle Java development team, Saint Petersburg, Russia
* I would love to change the world, but they won't give me the sources.
