RFR [11] 8198358 : Align organization of DualStackPlainSocketImpl with TwoStacksPlainSocketImp [win]

Chris Hegarty chris.hegarty at oracle.com
Fri Mar 9 11:50:44 UTC 2018


Ivan,

Thanks for breaking up the changes, it makes it easier to review.

I disagree with changing DualStackPlainSocketImp to conform with
TwoStacks (and unix/PlainSocketImpl). I would prefer to move the
latter to the former. Specifically, around the use of fdAccess.

-Chris.


On 06/03/18 20:31, Ivan Gerasimov wrote:
> In order to make is easier to review the fix, I made the webrev.ksh to 
> generate a series of incremental webrevs from the mq patch stack.
> 
> Here's the list of the incremental changes with a brief comments:
> 
> WEBREV-000: http://cr.openjdk.java.net/~igerasim/8198358/00/000/webrev/
>      Only changing the order of methods declaration
> 
> WEBREV-001: http://cr.openjdk.java.net/~igerasim/8198358/00/001/webrev/
>      Renaming initIDs to initProto
> 
> WEBREV-002: http://cr.openjdk.java.net/~igerasim/8198358/00/002/webrev/
>      Changing socketBind
> 
> WEBREV-003: http://cr.openjdk.java.net/~igerasim/8198358/00/003/webrev/
>      Changing socketCreate
> 
> WEBREV-004: http://cr.openjdk.java.net/~igerasim/8198358/00/004/webrev/
>      Changing socketListen
> 
> WEBREV-005: http://cr.openjdk.java.net/~igerasim/8198358/00/005/webrev/
>      Changin socketAvailable
> 
> WEBREV-006: http://cr.openjdk.java.net/~igerasim/8198358/00/006/webrev/
>      Changing socketClose0
> 
> WEBREV-007: http://cr.openjdk.java.net/~igerasim/8198358/00/007/webrev/
>      Changing socketShutdown
> 
> WEBREV-008: http://cr.openjdk.java.net/~igerasim/8198358/00/008/webrev/
>      Changing socketSendUrgentData
> 
> WEBREV-009: http://cr.openjdk.java.net/~igerasim/8198358/00/009/webrev/
>      Changing socketAccept
> 
> WEBREV-010: http://cr.openjdk.java.net/~igerasim/8198358/00/010/webrev/
>      Changing socketConnect
> 
> WEBREV-011: http://cr.openjdk.java.net/~igerasim/8198358/00/011/webrev/
>     Minor editing, comments, moving code
> 
> WEBREV-012: http://cr.openjdk.java.net/~igerasim/8198358/00/012/webrev/
>      Changing socketSetOption
> 
> WEBREV-013: http://cr.openjdk.java.net/~igerasim/8198358/00/013/webrev/
>      Changing socketGetOption
> 
> WEBREV-014: http://cr.openjdk.java.net/~igerasim/8198358/00/014/webrev/
>      Moving a few methods one more time
> 
> 
> Accumulative webrev with all the changes above is available here:
> http://cr.openjdk.java.net/~igerasim/8198358/01/webrev/
> 
> 
> Thanks in advance!
> 
> Ivan
> 
> On 3/1/18 8:43 PM, Ivan Gerasimov wrote:
>> Hello!
>>
>> I'd like to do the next step toward removing the TwoStacks socket 
>> implementation on Windows.
>>
>> It would be aligning the two implementations (DualStack and 
>> TwoStacks), so they can be easier merged together during the next step.
>>
>> There are three PlainSocketImpl implementations in JDK:
>> java.base/windows/classes/java/net/DualStackPlainSocketImpl.java
>> java.base/windows/classes/java/net/TwoStacksPlainSocketImpl.java
>> java.base/unix/classes/java/net/PlainSocketImpl.java
>>
>> While two later have very similar organization (in particular, set of 
>> native methods), the former is organized slightly differently.
>> In order to merge the two Windows implementation together, they first 
>> need to be organized in a similar way.
>> For consistency, DualStack implementation will be reorganized to be 
>> aligned with TwoStacks and unix/PlainSocketImpl.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8198358
>> Webrev: http://cr.openjdk.java.net/~igerasim/8198358/00/webrev/
>>
>> The change looks somewhat messy, but in fact it was a series of 
>> incremental changes, which I still keep in the mercurial 'mq'.
>>
>> (I wish the webrev could be made incremental based on the mq patches, 
>> to make it easier to review.)
>>
>> The patched JDK builds fine and all the regression tests pass Okay.
>>
>>
>> Thanks in advance!
> 


More information about the net-dev mailing list