> On 17 Oct 2016, at 09:40, Langer, Christoph <> wrote:
> …
> Hi,
> as I already mentioned, here is another proposal for cleanup in libnet:

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

> 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?


> 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.


> Thanks and best regards
> Christoph

Reply via email to