Hi all,

can I please get a review for my change: 
http://cr.openjdk.java.net/~clanger/webrevs/8160174.3/ ?

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.hega...@oracle.com>
> Cc: net-dev@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.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

Reply via email to