Thanks. Pushed:
  http://hg.openjdk.java.net/jdk9/dev/jdk/rev/ac3e32924dfb

-Chris.

> On 8 Jun 2016, at 15:07, Langer, Christoph <christoph.lan...@sap.com> wrote:
> 
> Hi Chris,
> 
> I now took time to look at your proposal based on ioctl calls. This looks 
> very good. I came up with some modifications to your patch and created 
> another webrev:
> 
> http://cr.openjdk.java.net/~clanger/webrevs/8158519.1/
> 
> This builds on Linux, AIX, Solaris and Mac and the basic test to list all 
> network interfaces on these platforms still succeeds. I also tested on Linux 
> that the bug reported by Doychin is fixed.
> 
> My changes in detail:
> - picked up the suggestions of Dmitry, except for the IFF_POINTOPOINT check 
> which I left untouched
> - corrected further indentations and removed a debug fprintf output which was 
> left
> - changed the order of arguments for function addif (you may or may not like 
> this, of course ;-))
> - renamed ifr_netmaskP to ifr_subnetaddrP as I think this fits more here 
> since the subnetaddr is converted to a netmask by computeMaskFromAddress 
> (which you may or may not like, as well)
> - AIX: need to use ifreqP->ifr_addr for SIOCGIFNETMASK since 
> ifreqP->ifr_netmask is not available on this platform
> 
> From my point of view this change could be pushed now.
> 
> As for the general discussion getifaddrs vs. ioctl: I would like to continue 
> refactoring the code ending up with 2 sets of enumInterfaces implementations. 
> One getifaddrs based and the other ioctl based. And then I would introduce 
> some #ifdef section which switches to the desired path per platform. I'll 
> come up with a proposal for that after this change got pushed.
> 
> Thanks and Best regards
> Christoph
> 
> 
>> -----Original Message-----
>> From: net-dev [mailto:net-dev-boun...@openjdk.java.net] On Behalf Of Dmitry
>> Samersoff
>> Sent: Donnerstag, 2. Juni 2016 19:35
>> To: Chris Hegarty <chris.hega...@oracle.com>; Doychin Bondzhev
>> <doyc...@dsoft-bg.com>; OpenJDK Network Dev list <net-
>> d...@openjdk.java.net>
>> Subject: Re: 8158519: Incorrect network mask and broadcast address on Linux
>> when there are multiple IPv4 addresses on an interface
>> 
>> Chris,
>> 
>> Looks good for me. Only minor nits (no need to re-review).
>> 
>> 1040 missed space before {
>> 
>> 1158 please, add comment and {} around continue.
>> 
>> 1938  I'm not sure that check for IFF_POINTOPOINT is necessary (but
>> don't mid to leave it). Do you know the case when both flags are set?
>> 
>> -Dmitry
>> 
>> On 2016-06-02 15:06, Chris Hegarty wrote:
>>> Doychin, et al,
>>> 
>>> I finally got time to look at the issue you reported and your suggested 
>>> patch. I
>> filed
>>> 8158519 [1] to track the issue. I think your suggested patch may be ok, but 
>>> I
>> just
>>> wanted to push on the ioctl approach. I found an alternative, and verified 
>>> that
>> it
>>> works as expected. Please take a look, and verify in your environment.  Then
>> we
>>> need to weigh up the two separate approaches.
>>> 
>>> http://cr.openjdk.java.net/~chegar/8158519.00/
>>> 
>>> For the record, I don’t have any specific issue with using getifaddrs, I 
>>> just
>> wanted to
>>> see if there was a less invasive change. That said, using getifaddrs could 
>>> lead
>> to
>>> cleaner code, but we would need to check the situation on AIX ( which I 
>>> don’t
>> have
>>> access to ).
>>> 
>>> -Chris.
>>> 
>>> [1] https://bugs.openjdk.java.net/browse/JDK-8158519
>>> 
> 

Reply via email to