Hi Chris, Bernd, thanks for your input.
I will remove the change from "java/net/SocketException” to JNU_JAVANETPKG. Splitting the refacturing of the Throw functions and the call site changes is generally ok for me. The question is, which way to go with the Throw functions. NET_ThrowByNameWithLastError as it is now has no benefit. It does the following: 279 char errmsg[255]; 280 sprintf(errmsg, "errno: %d, error: %s\n", errno, defaultDetail); 281 JNU_ThrowByNameWithLastError(env, name, errmsg); It obtains the errno and constructs a message that contains the errno and the handed defaultDetail. Then it calls JNU_ThrowByNameWithLastError with that message. But then JNU_ThrowByNameWithLastError retrieves the error message in a textual way by calling getLastErrorString and completely ignores the message that NET_ThrowByNameWithLastError calculated (assuming that getLastErrorString would probably always return some value). So I think we should keep JNU_ThrowByNameWithLastError with the current behavior, that is ignore the defaultMessage when an error string can be retrieved. And then there should be a JNU_ThrowByNameWithMessageAndLastError which would print both, the handed message and the error string. But which value would NET_ThrowByNameWithLastError have then? Just a wrapper for JNU_ThrowByNameWithLastError? As for error numbers: Those are currently only printed in the fallback case when getLastErrorString does not return anything useful. So, shall I add the error number to the message in any case? Best regards Christoph > -----Original Message----- > From: Chris Hegarty [mailto:chris.hega...@oracle.com] > Sent: Mittwoch, 1. Juni 2016 11:26 > To: Langer, Christoph <christoph.lan...@sap.com> > Cc: net-dev@openjdk.java.net; Bernd Eckenfels <e...@zusammenkunft.net> > Subject: Re: Ping - RFR 8158023: SocketExceptions contain too little > information > sometimes > > Christoph, > > > On 1 Jun 2016, at 08:33, e...@zusammenkunft.net wrote: > > > > Hello, > > > > Since nobody commented, here is my non binding comment: > > > > I am not sure which Exception call-site actually improved (i.e. added the > > error > code)? I would split out the improvement of this/those throws and the > refacturing of the code. > > Yes, please. > > > Personally I would prefer to keep the NET_ variant and adjust the native > utilities to remove code duplication. > > Also I dislike the change from "java/net/SocketException” to JNU_JAVANETPKG > “SocketException”. > My preference is to keep the former / original. > > > Does your new version still contain the nummric error code (os error > messages are unfortunatelly translated). > > -Chris. > > > Gruss > > Bernd > > -- > > http://bernd.eckenfels.net > > > > -----Original Message----- > > From: "Langer, Christoph" <christoph.lan...@sap.com> > > To: "net-dev@openjdk.java.net" <net-dev@openjdk.java.net> > > Sent: Mi., 01 Juni 2016 9:20 > > Subject: Ping - RFR 8158023: SocketExceptions contain too little information > sometimes > > > > Hi, > > > > ping - any comments on this? > > > > Thanks > > Christoph > > > > From: Langer, Christoph > > Sent: Freitag, 27. Mai 2016 10:30 > > To: net-dev@openjdk.java.net > > Cc: core-libs-...@openjdk.java.net; nio-...@openjdk.java.net > > Subject: RFR 8158023: SocketExceptions contain too little information > sometimes > > > > Hi all, > > > > please review the following change: > > Webrev: http://cr.openjdk.java.net/~clanger/webrevs/8158023.1/ > > Bug: https://bugs.openjdk.java.net/browse/JDK-8158023 > > > > During error analysis I stumbled over a place where I encountered a > SocketException which was thrown along with some strerror information as > message. I found it hard to find the originating code spot with that > information. > > > > So I looked at the places where we throw exceptions, namely JNU_Throw... > and NET_Throw... functions and came up with the following enhancement: > > - NET_ThrowByNameWithLastError can go completely as it does not provide > any benefit over JNU_ThrowByNameWithLastError. > > - JNU_ThrowByNameWithLastError can be cleaned up. > > > > - I added JNU_ThrowByNameWithMessageAndLastError to print out a string > like message + ": " + last error. > > > > - I went over all places where NET_ThrowByNameWithLastError is used and > replaced it appropriately. > > > > Do you think this change is desirable/possible? > > > > Though it's mainly a net topic, I'm posting it to nio-dev and core-libs-dev > > as > well as JNU_Throw... code affects all. > > > > Best regards > > Christoph > >
