8339414: Fix JDK-8202369 incorrect backport for 8u

[fitzsim]

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 (a62dd62) 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.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• JDK-8339414 needs maintainer approval

Issue

• JDK-8339414: Fix JDK-8202369 incorrect backport for 8u (Task - P4 - Requested)

Reviewers

• Andrew John Hughes (@gnu-andrew - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/661/head:pull/661
$ git checkout pull/661

Update a local copy of the PR:
$ git checkout pull/661
$ git pull https://git.openjdk.org/jdk8u-dev.git pull/661/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 661

View PR using the GUI difftool:
$ git pr show -t 661

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/661.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back fitzsim! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

[mlbridge]

Webrevs

• 01: Full - Incremental (0857914d)
• 00: Full (8d8bef80)

[gnu-andrew]

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 #elsecase). 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 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.

[gnu-andrew]

Thanks. Happy for this to go in now it's just the backport.

Please apply for approval

[openjdk]

⚠️ @fitzsim This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

[fitzsim]

/approval request This fix is ready.

[openjdk]

@fitzsim
8339414: The approval request has been created successfully.