[jdk8u-dev] RFR: 8339414: Fix JDK-8202369 incorrect backport for 8u

[Andrew John Hughes]

On Wed, 4 Jun 2025 20:44:19 GMT, Thomas Fitzsimmons <duke at openjdk.org> wrote:

> There is a large `#ifdef` in JDK 8's `Inet4AddressImpl.c`:
> 
> 
> #if defined(__GLIBC__) || (defined(__FreeBSD__) && (__FreeBSD_version >= 601104))
> #define HAS_GLIBC_GETHOSTBY_R   1
> #endif
> 
> #if defined(_ALLBSD_SOURCE) && !defined(HAS_GLIBC_GETHOSTBY_R)
> [... 261 lines ...]
> #else /* defined(_ALLBSD_SOURCE) && !defined(HAS_GLIBC_GETHOSTBY_R) */
> [... ~ 244 similar lines ...]
> #endif /* _ALLBSD_SOURCE */
> 
> 
> When the fix for https://bugs.openjdk.org/browse/JDK-8201369 was backported to `8`, the second branch of the large `#ifdef` in `Inet4AddressImpl.c` was not changed at the same time, which is incorrect, since reverse lookup should only ever happen on `Solaris`, according to the description of `JDK-8201369`.
> 
> I used the `JDK 11` `unix/native/libnet/Inet4AddressImpl.c` implementation; after this change, the only difference between `11`'s `Java_java_net_Inet4AddressImpl_getLocalHostName` and the `defined(HAS_GLIBC_GETHOSTBY_R)` one will be the use of `JVM_GetHostName` in `8` versus `gethostname` in `11`.
> 
> I decided to update the `hostlen` argument to `getnameinfo` from `NI_MAXHOST` to `sizeof(hostname)` to fix https://bugs.openjdk.org/browse/JDK-8202181 for this function.  I do not have a `Solaris` machine to test on, but this matches `11`'s `getnameinfo` calls, and is correct given `hostname[NI_MAXHOST] = '\0'`.
> 
> The resulting behaviour should be the same, other than the reverse lookup only being performed on `Solaris`.
> 
> The implementations of `Java_java_net_Inet4AddressImpl_getLocalHostName` in each of the `#ifdef` branches diverge already because the fix for https://bugs.openjdk.org/browse/JDK-7112670 (https://github.com/openjdk/jdk8u-dev/commit/a62dd6282b7a0f1f3c75dad90385783b93d30e61) was only applied to the second branch.
> 
> Probably the upper `Java_java_net_Inet4AddressImpl_getLocalHostName` implementation could be made identical to the lower, but I do not want to do that, just in case `NI_MAXHOST` is preferred to `sizeof(hostname)` on `!defined(HAS_GLIBC_GETHOSTBY_R)` operating systems.

Yes, this seems like a clear case of the duplication of `getLocalHostName(JNIEnv *env, jobject this)` being missed in backporting from 11u (which has a much simpler addition of the `#ifdef` and the `#else`case). There's a lot of noise in removing the stored `error` value both in the original 8u backport and this change, but the overall change amounts to the same as 11u.

To that end, I don't think a partial backport of [JDK-8202181](https://bugs.openjdk.org/browse/JDK-8202181) belongs here. Even if we agree not to alter the branch without `HAS_GLIBC_GETHOSTBY_R` (which I think has merit as we don't define `NI_MAXHOST` ourselves there), we should fix `getHostByAddr` and the IPv6 implementation. So I'd rather see that handled in full as a separate PR, rather than deviating from the 8202369 backport here.

-------------

Changes requested by andrew (Reviewer).

PR Review: https://git.openjdk.org/jdk8u-dev/pull/661#pullrequestreview-2902286336