On 11/04/14 10:27, Chris Hegarty wrote:
This looks mainly good to me.
Just a few small comments (mainly on the use of reflection):
1) jdk/net/Sockets.java
You could use a SharedSecret to access the private methods in the
public java.net package, but reflection is ok too.
I think the amount of reflection glue is not too much. So, I'd prefer
to stick with that in this instance.
2) If you stick with reflection then if a lookup of any of the methods
fails, throws ReflectiveOperationException, then you should probably
throw InternalError as there is something badly wrong.
okay, good idea.
3) Would it make sense for the invoker static methods of
jdk.net.Sockets to throw UncheckedIOException(e) ??
I'd prefer to throw IOException when it is an IOException.
I'm open to suggestion for when it's not an IOException.
I know RuntimeException is vague, but then again UncheckedIOException
doesn't seem right either to me, since it's something other
than an io exception in this situation.
Thanks,
Michael
-Chris.
On 10/04/14 18:13, Michael McMahon wrote:
Hi,
This is the webrev for the 8u20 version of the fix that was reviewed
yesterday for 9.
JDK
===
http://cr.openjdk.java.net/~michaelm/8036979.8u20/jdk/01/webrev/
Top repo
=====
http://cr.openjdk.java.net/~michaelm/8036979.8u20/top/01/webrev/
The good news is that the change is almost the same as the JDK 9 version
with the following differences:
1) The java.net public API changes are gone. The new public methods
for 9
in SocketImpl and DatagramSocketImpl are package private here.
2) A new package private class java.net.SocketsUtil acts as a bridge
between
the public API in jdk.net.Sockets and the implementation in
java.net
3) jdk.net.Sockets uses reflection to access the methods of
java.net.SocketsUtil
4) The test of the public java.net API is gone and the other test
augmented
with some additional tests for the standard socket options
Thanks,
Michael
