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

> On 27 Mar 2018, at 18:30, Ivan Gerasimov wrote: > > Thank you Christoph and Chris for the review! > > I made the following suggested changes: > - fdAccess made private static final in both TwoStacks and DualStack. > - removed redundant check IS_NULL(iaObj) at bind0

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

Thank you Christoph and Chris for the review! I made the following suggested changes: - fdAccess made private static final in both TwoStacks and DualStack. - removed redundant check IS_NULL(iaObj) at bind0 (indeed, the address was already checked at Java level). For now, I would prefer to

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

Ivan, > On 26 Mar 2018, at 23:08, Ivan Gerasimov wrote: > > Hello everyone! > > A few modifications were done to the patch. > Below is the list of updated parts of the patch with comments about what has > changed, comparing to the previous version of the patch. > >

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

Behalf Of > Ivan Gerasimov > Sent: Dienstag, 27. März 2018 00:08 > To: Chris Hegarty <chris.hega...@oracle.com>; net-dev@openjdk.java.net; > Alan Bateman <alan.bate...@oracle.com> > Subject: Re: RFR [11] 8198358 : Align organization of > DualStackPlainSocketImpl with TwoS

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

On 3/25/18 12:11 PM, Alan Bateman wrote: On 25/03/2018 19:13, Ivan Gerasimov wrote: : In the code above, newfd was obtained from a call to accept() a few lines before this check. So, the Java code has no way of being aware of this socket, and it will never be closed unless we do it right

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

Hello everyone! A few modifications were done to the patch. Below is the list of updated parts of the patch with comments about what has changed, comparing to the previous version of the patch. The patched JDK builds fine and all the tests pass well. WEBREV-000:

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

On 25/03/2018 19:13, Ivan Gerasimov wrote: : In the code above, newfd was obtained from a call to accept() a few lines before this check. So, the Java code has no way of being aware of this socket, and it will never be closed unless we do it right here, before returning -1. The

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

Hi Chris! On 3/25/18 6:37 AM, Chris Hegarty wrote: On 23 Mar 2018, at 21:55, Chris Hegarty > wrote: For reference, here's the combined webrev: http://cr.openjdk.java.net/~igerasim/8198358/03/webrev/

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

> On 23 Mar 2018, at 21:55, Chris Hegarty wrote: > >> For reference, here's the combined webrev: >> http://cr.openjdk.java.net/~igerasim/8198358/03/webrev/ >> > > I think this is good. And thanks for the

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

Ivan, Thank you for persisting with this. > On 23 Mar 2018, at 19:09, Ivan Gerasimov wrote: > > Hello! > > I reverted the direction of the patch, and now TwoStacks implementation is > changed to mimic DualStack. > As part of this fix, I also added another @run line

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

Hello! I reverted the direction of the patch, and now TwoStacks implementation is changed to mimic DualStack. As part of this fix, I also added another @run line to several tests to execute them with the option -Djava.net.preferIPv4Stack=true to make sure both implementations work as

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

Hi Chris! Thank you for reviewing the patch set! On 3/9/18 3:50 AM, Chris Hegarty wrote: 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

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

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,

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

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

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

Thank you Christoph for reviewing this! On 3/6/18 4:53 AM, Langer, Christoph wrote: Hi Ivan, I went through the changes a bit and I would think it is really a good cleanup and will make the future merge of the implementations easier. One thing I saw was that TwoStacksPlainSocketImpl.c has

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

Hi Ivan, I went through the changes a bit and I would think it is really a good cleanup and will make the future merge of the implementations easier. One thing I saw was that TwoStacksPlainSocketImpl.c has an #include in line 25. I think that can be removed!? I had problems importing the