Ping: RFR (M): 8167481: cleanup of headers and includes for native libnet

[Chris Hegarty]

Christoph,

> On 17 Oct 2016, at 09:40, Langer, Christoph <christoph.langer at sap.com> wrote:
> …
> Hi,
>  
> as I already mentioned, here is another proposal for cleanup in libnet:
> https://bugs.openjdk.java.net/browse/JDK-8167481
> http://cr.openjdk.java.net/~clanger/webrevs/8167481.0/

Overall, this looks good. Specific comments below ...

> This time I touched the includes. Generally I reordered the includes to include “net_util.h” first, then any JNI includes, such as “java_net_InetAddress.h” and finally all standard header includes in alphabetical order. For platform specific includes, I use the order Linux, AIX, Solaris, BSD. I removed all includes that don’t seem to be necessary to get the respective file compiled. Is that the correct approach that is desired?

It is good to be consistent. My personal preference is to order the includes;
1) system, 2) platform specific includes, 3) JNI includes, and finally, 4) 
generated headers ( e.g. “java_net_InetAddress.h” ).

> To make this work, I had to remove the definitions “IPv4” and “IPv6” in net_util.h and replace their usages with “java_net_InetAddress_IPv4” resp. “java_net_InetAddress_IPv6” from JNI which seems to be the correct thing to do anyway.

Right. This is a good change. Trivially, and it is not strictly necessary but
good for readability, would be to add @Native to InetAddress.IPv4 and
InetAddress.IPv6.

> For Windows I also cleaned up (removed) the definition of SOCKADDR_IN6 in net_util_md.h and replaced all occurences with sockaddr_in6. It seems that the capital letters version SOCKADDR_IN6 is a remainder of a very old Microsoft runtime (_MSC_VER < 1310) which corresponds to MSVC 7.0, which is even older than Visual Studio 2003. So I’m wondering if I should remove the whole section in windows net_util_md.h now (lines 47 – 177)?

I think it should be ok to remove this.

> For Windows it seems that we can also completely get rid of src/java.base/windows/native/libnet/icmp.h as the icmp stuff is all brought by the #include <icmpapi.h>. Would you agree to that?

Ok.

> So far I successfully ran builds in my local environments for Windows, Linux, AIX, Solaris and Apple. I’ve now added the changeset to our local regression testing environment for OpenJDK. If the change is to your like, I would like to ask you to also run it through JPRT to see if I missed some dependency.

I ran your patch through our internal build and test system and there
are no surprises.

-Chris.

> Thanks and best regards
> Christoph