Ping - RFR 8158023: SocketExceptions contain too little information sometimes

[Langer, Christoph]

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.hegarty at oracle.com]
> Sent: Mittwoch, 1. Juni 2016 11:26
> To: Langer, Christoph <christoph.langer at sap.com>
> Cc: net-dev at openjdk.java.net; Bernd Eckenfels <ecki at zusammenkunft.net>
> Subject: Re: Ping - RFR 8158023: SocketExceptions contain too little information
> sometimes
> 
> Christoph,
> 
> > On 1 Jun 2016, at 08:33, ecki at 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.langer at sap.com>
> > To: "net-dev at openjdk.java.net" <net-dev at 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 at openjdk.java.net
> > Cc: core-libs-dev at openjdk.java.net; nio-dev at 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
> >