Thanks, I pushed: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/2b5229c75e93
Best regards Christoph > -----Original Message----- > From: Chris Hegarty [mailto:chris.hega...@oracle.com] > Sent: Dienstag, 27. September 2016 21:24 > To: Langer, Christoph <christoph.lan...@sap.com> > Cc: net-dev@openjdk.java.net > Subject: Re: RFR(XS): 8166584: Remove obsolete utility function > NET_ThrowSocketException in windows libnet > > > > On 27 Sep 2016, at 19:56, Langer, Christoph <christoph.lan...@sap.com> > wrote: > > > > Chris, > > > > thanks for your input. > > > > If there's no objections I'd push it like this later tomorrow: > > http://cr.openjdk.java.net/~clanger/webrevs/8166584.2/ > > Looks ok to me Christoph. > > Thanks, > -Chris. > > > I've replaced the JNU_JAVANETPKG and JNU_JAVAIOPKG macros with the full > exception class names. > > > > Best regards > > Christoph > > > >> -----Original Message----- > >> From: Chris Hegarty [mailto:chris.hega...@oracle.com] > >> Sent: Dienstag, 27. September 2016 10:10 > >> To: Langer, Christoph <christoph.lan...@sap.com> > >> Cc: net-dev@openjdk.java.net > >> Subject: Re: RFR(XS): 8166584: Remove obsolete utility function > >> NET_ThrowSocketException in windows libnet > >> > >> Christoph, > >> > >> On 26 Sep 2016, at 18:58, Langer, Christoph <christoph.lan...@sap.com> > >> wrote: > >>> > >>> 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/ > >> > >> This looks fine. > >> > >>> 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? > >> > >> I don’t believe that using a GlobalRef is worth it here. It adds a little > >> complication, while not offering much benefit. JNU_ThrowByName > >> should be fine. > >> > >>> 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? > >> > >> My preference is to remove JNU_JAVANETPKG, and just use "java/net/“. > >> > >> -Chris > >> > >>> 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- > >> d...@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. > >
