Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

Hi Alan, Thanks for your review. Sounds good, will remove the file before pushing and use as test for JDK-8242885 instead. Kind regards, Patrick On 07/05/2020 10:38, Daniel Fuchs wrote: Hi Alan, On 07/05/2020 07:59, Alan Bateman wrote: I'd prefer to see JDK-8242885 fixed and SetGetSendB

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

Hi Alan, On 07/05/2020 07:59, Alan Bateman wrote: I'd prefer to see JDK-8242885 fixed and SetGetSendBufferSize/testInitialSendBufferSize updated to test a maximally sized IPv6 packet (we should probably have done that in advance of the JEP). OK - let's remove the SendBufCheck before pushing.

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

On 06/05/2020 21:27, Patrick Concannon wrote: Hi Alan, With regards to the SetGetSendBufferSize.java test: yes, it was created to increase the test coverage for the DatagramSocket class. However, it was decided that it should be handled separately and was pushed to the mainline in advance of

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

Hi Alan, With regards to the SetGetSendBufferSize.java test: yes, it was created to increase the test coverage for the DatagramSocket class. However, it was decided that it should be handled separately and was pushed to the mainline in advance of this RFR - (JDK-8243488

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

On 15/04/2020 20:22, Patrick Concannon wrote: : WRT the PDSI issue, I've created a bug to track this and have assigned it to myself. You can view the issue here: https://bugs.openjdk.java.net/browse/JDK-8242885 Thanks. That should be trivial to fix and should allow SendBufCheck to be updat

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

Hi Alan, Thanks for your further comments. I've refactored DatagramSocket as you suggested, and returned DatagramSocketAdaptor to the correct formatting. I also removed the incorrect @bug tag from SetGetSendBufferSize and /othervm from the other test updates.I've included these changes in the

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

On 14/04/2020 10:20, Daniel Fuchs wrote: That's where I disagree because it's a slight difference in behavior between the old implementation and the new - so I believe it's worth having a test for it (even if does test the presence of a bug in the old impl). I like this test because it does s

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

Hi Alan, On 12/04/2020 16:51, Alan Bateman wrote: I don't think SetGetSendBufferSize should be in the webrev. It has @bug 8239355 so it's for a different issue. Patrick will confirm but IIRC this test was added because coverage testing reported that DS::setSendBufferSize was never called (pres

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

On 09/04/2020 12:26, Patrick Concannon wrote: Hi, Alan - I've gone through your comments and refactored the code accordingly. Just a few minor comments on the implementation changes. DatagramSocket.java - this version of the webrev has a block comment "A global switch ..." that looks like

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

Hi, Alan - I've gone through your comments and refactored the code accordingly. I spoke with Daniel about the SendBufCheck test, and he wants to keep this in the JEP as it allows to compare the behavior of all the different implementations in one place. Chris - Thanks for opening issue JDK-8

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

> On 31 Mar 2020, at 18:46, Alan Bateman wrote: > > On 31/03/2020 18:27, Chris Hegarty wrote: >> : >> - In DatagramSocket::createDelegate, "enable broadcast if possible” - >> Possibly due to refactoring, but I cannot reconcile this with the old >> implementation. > DatagramSocket is specified

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

On 31/03/2020 18:27, Chris Hegarty wrote: : - In DatagramSocket::createDelegate, "enable broadcast if possible” - Possibly due to refactoring, but I cannot reconcile this with the old implementation. DatagramSocket is specified to make a best attempt to enable this option so I think it's in

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

> On 30 Mar 2020, at 19:27, Patrick Concannon > wrote: > ... > > http://cr.openjdk.java.net/~pconcannon/8241072/webrevs/webrev.01/ This looks very good, a testament to all the prior cleanup work that has already been done. Just a few comments: - DatagramSocket::getChannel now stands out a

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

On 30/03/2020 19:27, Patrick Concannon wrote: Hi Alan, Thanks for your feedback. I've incorporated your comments into the revised webrev below. http://cr.openjdk.java.net/~pconcannon/8241072/webrevs/webrev.01/ With regards to the UnreferencedXXX tests, I can take a look at these separate

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

Hi Alan, Thanks for your feedback. I've incorporated your comments into the revised webrev below. http://cr.openjdk.java.net/~pconcannon/8241072/webrevs/webrev.01/ With regards to the UnreferencedXXX tests, I can take a look at these separately. -Patrick On 23/03/2020 18:59, Alan Bateman

Re: RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

On 23/03/2020 16:37, Patrick Concannon wrote: This is a request for review for the implementation changes for JEP 373: 'Reimplement Legacy DatagramSocket API' [1]. A CSR has also been created [2]. The implementation of JEP 373 can be viewed on the JDK-8230211-branch in the sandbox [3]. A li

RFR[8241072]: 'Reimplement Legacy DatagramSocket API'

This is a request for review for the implementation changes for JEP 373: 'Reimplement Legacy DatagramSocket API' [1]. A CSR has also been created [2]. The implementation of JEP 373 can be viewed on the JDK-8230211-branch in the sandbox [3]. A link to the webrev with the cumulated changes is