Hi Chris, I agree with your comment on the NPE. It would probably be wrong. So I restored the old code and also removed the comments suggesting the NPE. Here is my new webrev: http://cr.openjdk.java.net/~clanger/webrevs/8166584.1/
What I was thinking a bit more about after I posted my initial webrev was the fact that the old NET_ThrowSocketException would register a GlobalRef to java/net/SocketException whereas the other, more generic code would always use the lookup by name. Would you think it is a performance benefit to keep a reference to a standard exception class in some place and use it for throwing or is it better to always look up the class? Throwing those kind of exceptions is probably not on the hot path anyway - but on the other hand it should be no issue to keep references to these very basic class types. What's your view on that? And another probably aesthetic thing: I notice that sometimes a JNU_JAVANETPKG "SocketException" is used and sometimes a "java/net/SocketException", even within the same file like SocketInputStream.c. Maybe I should unify this in the files that I touch here and if yes, shall I use the literal name or the JNU_JAVANETPKG define? Any opinion on that? Thanks for taking care of this, Christoph > -----Original Message----- > From: Chris Hegarty [mailto:chris.hega...@oracle.com] > Sent: Montag, 26. September 2016 16:51 > To: Langer, Christoph <christoph.lan...@sap.com>; net-dev@openjdk.java.net > Subject: Re: RFR(XS): 8166584: Remove obsolete utility function > NET_ThrowSocketException in windows libnet > > Christoph, > > On 22/09/16 21:59, Langer, Christoph wrote: > > Hi, > > > > while looking at utility functions for creating exceptions in > > libjava/libnet I found a small spot that should be consolidated right away. > > > > > > The function NET_ThrowSocketException does only exist in the windows > > native implementation and is only used in 3 places in > > SocketInputStream.c. I removed this in favor of directly calling > > JNU_ThrowByName as the Unix variant of that code already does. > > > > > > In that function Java_java_net_SocketInputStream_socketRead0 I also > > replaced throwing a SocketException with throwing an NPE in the rare > > case that a the JNI input for the file descriptor is null. That's > > probably more natural and should virtually never occur anyways. > > Hmmm... I'm not sure about this. SocketException is thrown on > unix too for a similar situation. More significantly, a null value > represents that the socket has been, possibly asynchronously, > closed. > > > Bug: https://bugs.openjdk.java.net/browse/JDK-8166584 > > > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8166584.0/ > > Other than the above concern, the remainder of the code looks ok > to me. > > -Chris.
