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