Hi, could somebody please have a quick look at this change. It's really not that complicated as it looks like from the comments - I just didn't manage to write it up in a more concise way :)
Thanks, Volker On Thu, Oct 16, 2014 at 4:39 PM, Volker Simonis <volker.simo...@gmail.com> wrote: > Hi, > > could you please hava a look at the following change: > > http://cr.openjdk.java.net/~simonis/webrevs/8060470.v1 > https://bugs.openjdk.java.net/browse/JDK-8060470 > > It's probably easier to read the following in the webrev, but I copy > it below for discussion. > > Regards, > Volker > > So here comes the first version of this change. Its main focus is the > unification and simplification of > Inet{4,6}AddressImpl_getLocalHostName() (notice that these native > methods are currently only called from InetAddress.getLocalHost() so > the impact is manageable): > > - Simplification: the current implementation has three versions of > this function: two versions of Inet4AddressImpl_getLocalHostName() > (one for "_ALLBSD_SOURCE && !HAS_GLIBC_GETHOSTBY_R" and another one > for the other Unix versions) and one version of > Inet6AddressImpl_getLocalHostName(). All these functions are very > similar and can be easily factored out into one new method. > - Unification: there are subtle and probably unnecessary differences > between the IPv4 and IPv6 version of these methods which can be easily > eliminated. > > The only difference between the two IPv4 versions was the ai_family > flag passed as hint to the getaddrinfo() call. The Mac version used > AF_UNSPEC while the general Unix version used AF_INET. I don't see a > reason (and my tests didn't show any problems) why we couldn't use > AF_INET on MacOS as well. > > The IPv6 version used AF_UNSPEC as well. The new refactored method > getLocalHostName() which is now called from both, the single instance > of Inet4AddressImpl_getLocalHostName() and > Inet6AddressImpl_getLocalHostName() uses AF_INET in the IPv4 case > (which shouldn't change anything) and AF_INET6 for the IPv6 case. > Additionally, it uses the flag AI_V4MAPPED in the IPv6 case. This will > return an IPv4-mapped IPv6 addresses if no matching IPv6 addresses > could be found. > > The last difference between the old IPv4 and IPv6 versions was the > fact that the IPv4 versions always did a reverse lookup for the host > name. That means that after querying the hostname with gethostname(), > they used a call to getaddrinfo() to get the IP address of the host > name and finally they called getnameinfo() on that IP address to get > the host name once again. The IPv6 version only did this reverse > lookup on Solaris. > > It is unclear why this reverse lookup was necessary at all. Especially > if we take into account that the resulting host name will be only used > in InetAddress.getLocalHost() where it will finally serve as input to > InetAddressImpl.lookupAllHostAddr() which will in turn do exactly the > same reverse lookup procedure (at least if the default name service is > used). Therefore the new implementation switches this reverse lookup > off by default unless the user requests it with the two system > properties java.net.doIPv4ReverseLookupInGetLocalHost and > java.net.doIPv6ReverseLookupInGetLocalHost for IPv4 and IPv6 > respectively. Consequently, the new Unix version of > Java_java_net_Inet{4,6}AddressImpl_getLocalHostName is now equal to > its Windows counterpart which simply returns the result of > gethostname() as well. > > Notice that for these properties, the name "IPv4" and "IPv6" refer to > the actual InetAddressImpl version (i.e. either Inet4AddressImpl or > Inet6AddressImpl) that is used by InetAddress. On most modern hosts, > InetAddress will use an Inet6AddressImpl helper if the host is IPv6 > "capable". That doesn't necessarily mean that > InetAddress.getLocalHost() will return an IPv6 address or even that > there's a network interface with an IPv6 address - it just means that > the network stack can handle IPv6. InetAddress can be forced to use > the Inet4AddressImpl helper by setting the java.net.preferIPv4Stack > property to false. > > In the new implementation both properties > java.net.doIPv4ReverseLookupInGetLocalHost and > java.net.doIPv6ReverseLookupInGetLocalHost are set to false by > default. But the old behavior could be restored by setting > java.net.doIPv4ReverseLookupInGetLocalHost=true and > java.net.doIPv6ReverseLookupInGetLocalHost=true (Solaris only). > > In a previous mail thread [2] we discussed the possibility of > replacing the call to getnameinfo() in > Inet{4,6}AddressImpl_getLocalHostName() by simply using the > ai_canonname field returned by the getaddrinfo() call. But because I > think the reverse lookup is unnecessary anyway (and disabled by > default in the new implementation), I didn't tried that any more and > stayed with the old version. > > I've built these changes on Linux, Solaris, MacOS X and AIX and > manually tested them on various hosts with different IPv4/IPv6 setups > without any problems. In cases where the output of > InetAddress.getLocalHost() differed in the new version this was only a > difference in the hostname part of the InetAddress (i.e. fully > qualified name vs. simple name) and this could be easily restored with > the help of the new system properties. Notice that for the new version > the host name now usually corresponds to what the hostname command > returns on Unix which is probably what most people would expect. In > the old version the host name was more dependent on the local system > configuration (i.e. /etc/hosts or /etc/nsswitch.conf) as discussed in > [2]. > > The following mail threads already discuss this issue: > > [1] > http://mail.openjdk.java.net/pipermail/net-dev/2014-October/thread.html#8721 > [2] http://mail.openjdk.java.net/pipermail/net-dev/2013-June/thread.html#6543 > > PS: We probably shouldn't discuss this too long as there are other > methods like Java_java_net_Inet{4,6}AddressImpl_lookupAllHostAddr > which are waiting for unification and simplification as well :)
