Hi Chris, ok, here is my new version which I think is quite a nice cleanup - though probably not "S" any more:
Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8160174.3/ Bug: https://bugs.openjdk.java.net/browse/JDK-8160174 I updated the bug report to list all the small things that I've fixed. I now took the approach to depuzzle all "enum*Interfaces" functions for the platforms and now the coding should really be easier to read - though, of course, some parts got duplicated. The notable differences in the output of NetworkInterface.getAll() are that a) no broadcast address is returned any more for loopback addresses on Linux and b) subnet prefixes for AIX IPv6 interfaces should work now. The rest should be optimizations under the cover. Please review. Thanks Christoph > -----Original Message----- > From: Chris Hegarty [mailto:chris.hega...@oracle.com] > Sent: Dienstag, 12. Juli 2016 16:10 > To: Langer, Christoph <christoph.lan...@sap.com> > Cc: net-dev@openjdk.java.net > Subject: Re: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and > improvements for network interface listing > > Christoph, > > > On 11 Jul 2016, at 14:36, Langer, Christoph <christoph.lan...@sap.com> > wrote: > > > > Hi Chris (or anyone who is holding a stake in the NetworkInterface native > implementation), > > > > I’ve spent some time on cleaning up NetworkInterface.c and came up with a > bigger change:http://cr.openjdk.java.net/~clanger/webrevs/8160174.2/ > > > > With this I attempted to consolidate the interface listing functionality of > > the 2 > main categories – one is ioctl (used on Linux IPv4, Solaris and AIX) and the > other is getifaddrs (used on MacOS). I introduced some defines to switch > between the implementations. I also consolidated the functionality for the > ioctl > based network interface listings by using an #ifdef section to distinguish > between the Linux/AIX versus Solaris field and constant names. > > > > I’ve spent some time testing on the platforms and in principal it works. > > But as > it is a matter of taste, I’d like to ask you if you support this type of > change or > have any hints or recommendations before going forward to finalize this. > > I’m personally don’t like this approach much. I think it adds further > complexity and difficulty to this code, which already has its fair share > of both. > > > For Linux I’m also suggesting to use the getifaddrs approach – I tested and > found it working everywhere and with this we could get rid of the > implementation to read /proc/net for IPv6. > > You should probably break this type of change out, so that it can be > evaluated independently, on its own merit. If it add nothing more than > clean up, may it makes sense for this to happen early in 10, where it > can have a longer time to bake. > > > Furthermore I’m generally setting Null for the IPv6 broadcast address – > > which > I think is common sense for IPv6. > > Same as above. This “smaller” changes can sometimes get lost in > the noise of larger refactoring. > > -Chris. > > > > Thanks in advance and best regards > > Christoph > > > > > > From: Langer, Christoph > > Sent: Donnerstag, 23. Juni 2016 16:37 > > To: 'net-dev@openjdk.java.net' <net-dev@openjdk.java.net> > > Subject: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and > improvements for network interface listing > > > > Hi, > > > > can I please get a review the following change: > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8160174.1/ > > Bug: https://bugs.openjdk.java.net/browse/JDK-8160174 > > > > I made further fixes and cleanups for listing Unix type network interfaces, > especially on AIX and Linux. I ran builds and verified also on Solaris and > Mac. > > > > Thanks > > Christoph
