PING: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and improvements for network interface listing

[Langer, Christoph]

Hi Chris,

thanks, I pushed it: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/a8db670c7d12

After giving it a certain while in JDK9 I will also want to downport it to JDK8, but let's wait a bit first...

Best regards
Christoph

> -----Original Message-----
> From: Chris Hegarty [mailto:chris.hegarty at oracle.com]
> Sent: Donnerstag, 21. Juli 2016 17:15
> To: Langer, Christoph <christoph.langer at sap.com>; net-dev at openjdk.java.net
> Subject: Re: PING: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and
> improvements for network interface listing
> 
> Hi Christoph,
> 
> On 19/07/16 14:48, Langer, Christoph wrote:
> > Hi all,
> >
> > can I please get a review for my change:
> http://cr.openjdk.java.net/~clanger/webrevs/8160174.3/ ?
> 
> This really hard to review, since so many parts of the code
> are moving around, but we did put this off a few times
> already so probably makes sense to do it now. The code
> should be more readable afterwards are as such easier to
> maintain.
> 
> I ran it through our internal build and test system and there
> were no surprises.  Consider it reviewed.
> 
> -Chris.
> 
> > I made some minor updates in place, mostly formatting, to the version from
> one week ago. The only real change I made is that I now set the scope id of IPv6
> addresses to the interface index also on BSD, where it was not done before.
> Here I have the question if this is really the desired behavior to always set the
> interface as scope of any IPv6 address?
> >
> > Thanks in advance
> > Christoph
> >
> >
> >> -----Original Message-----
> >> From: Langer, Christoph
> >> Sent: Mittwoch, 13. Juli 2016 17:02
> >> To: 'Chris Hegarty' <chris.hegarty at oracle.com>
> >> Cc: net-dev at openjdk.java.net
> >> Subject: RE: RFR (S) JDK-8160174: java.net.NetworkInterface - fixes and
> >> improvements for network interface listing
> >>
> >> 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.hegarty at oracle.com]
> >>> Sent: Dienstag, 12. Juli 2016 16:10
> >>> To: Langer, Christoph <christoph.langer at sap.com>
> >>> Cc: net-dev at 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.langer at 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 at openjdk.java.net' <net-dev at 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
> >