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
separately.
I skimmed through webrev.01 and just have a few comments:
NetMulticastSocket.checkOldImpl has been updated to check if the impl is
null, I assume that is not needed as it is now checked in the constructor.
DatagramSocket.delegate() - I assume the exception message should be
"Should not get here". An alternative here would be to just assert that
delegate != null. Either is okay.
DatagramSocket L139-142. This comment/Note seems to be "notes to self"
for possible future changes and maybe it should be removed to avoid
confusing readers.
DatagramSocket.createDelegate L1144 can be simplified to "if
(!initialized && delegate != null)".
DatagramSocketAdaptor uses 4-space indent rather than 8 so probably best
to keep that style consistent if you can.
The webrev still has SendBufCheck. That is a test for another issue so
I'll ignore this for now.
The rest looks good and a separate issue to replace the UnreferencedXXX
tests is okay with me.
-Alan.
