RFR 8158023: SocketExceptions contain too little information sometimes

Wed Jun 29 21:18:21 UTC 2016

Hi Roger,

thanks for reviewing.

As for:
> jni_util.c: line 216:
There I don't create an extra String but the Exception Object to throw, similar to the old function JNU_ThrowByNameWithLastError.

> jni_util.h:
>
>    line 117-119, The original comment was just as informative as the
I think you are right - I will restore the old comment.

If no objections I consider this reviewed and will push it tomorrow with the reverted comment lines.

Thanks
Christoph

> -----Original Message-----
> From: nio-dev [mailto:nio-dev-bounces at openjdk.java.net] On Behalf Of Roger
> Riggs
> Sent: Mittwoch, 29. Juni 2016 20:20
> To: nio-dev at openjdk.java.net
> Subject: Re: RFR 8158023: SocketExceptions contain too little information
> sometimes
>
> Hi Christoph,
>
> Looking good, its unfortunate that the handling of mixed platform and
> utf string require jni up calls to invoke java methods.
>
> jni_util.c: line 216:
>
>    You should not need to create an extra string; the string s is
> non-null and ready to use.
>
> jni_util.h:
>
>    line 117-119, The original comment was just as informative as the
> replacement.
>
> The rest looks fine.
>
> Roger
>
> On 6/28/16 4:45 PM, Langer, Christoph wrote:
> >
> > Hi Paul,
> >
> > Ok, you kind of got me convinced and it is also a quite simple
> > modification. I changed from “java.net.SocketException: ioctl
> > SIOCGSIZIFCONF failed: Bad file number” to “java.net.SocketException:
> > Bad file number (ioctl SIOCGSIZIFCONF failed)” like you suggested.
> >
> > The update is in place:
> > http://cr.openjdk.java.net/~clanger/webrevs/8158023.3/
> > <http://cr.openjdk.java.net/%7Eclanger/webrevs/8158023.3/>
> >
> > Now I finally need a review…
> >
> > Best regards
> >
> > Christoph
> >
> > *From:*Paul Benedict [mailto:pbenedict at apache.org]
> > *Sent:* Montag, 27. Juni 2016 18:15
> > *To:* Langer, Christoph <christoph.langer at sap.com>
> > *Cc:* Kenji Kazumura <kzr at jp.fujitsu.com>; Chris Hegarty
> > <chris.hegarty at oracle.com>; nio-dev at openjdk.java.net;
> > core-libs-dev at openjdk.java.net; net-dev at openjdk.java.net
> > *Subject:* Re: RFR 8158023: SocketExceptions contain too little
> > information sometimes
> >
> > Christoph, I didn't understand your explanation on your message
> > preference. Typically root cause information is printed last, not
> > first. Another reason not to change the ordering of the exception
> > message is that applications may be looking at existing strings. For
> > this example, if I may presume "Bad file number" is an existing
> > message, I would defer to the possibility applications may be exist
> > that test for that message condition.
> >
> >
> > Cheers,
> > Paul
> >
> > On Mon, Jun 27, 2016 at 2:42 AM, Langer, Christoph
> > <christoph.langer at sap.com <mailto:christoph.langer at sap.com>> wrote:
> >
> >     Hi,
> >
> >     eventually here is the - hopefully final - version of this fix:
> >     http://cr.openjdk.java.net/~clanger/webrevs/8158023.3/
> >     <http://cr.openjdk.java.net/%7Eclanger/webrevs/8158023.3/>
> >
> >     Now I leave JNU_ThrowByNameWithLastError untouched and I've also
> >     added conversion to the new function
> >     JNU_ThrowByNameWithMessageAndLastError. I've replaced
> >     JNU_ThrowByNameWithLastError with
> >     JNU_ThrowByNameWithMessageAndLastError in the java/net coding
> >     where I think it is appropriate (mostly in occasions when a
> >     SocketException is thrown kind of generically). @Paul: thanks for
> >     your suggestion regarding the output format but I would still
> >     prefer an output like "java.net.SocketException: ioctl
> >     SIOCGSIZIFCONF failed: Bad file number" vs. "
> >     java.net.SocketException: Bad file number (ioctl SIOCGSIZIFCONF
> >     failed)" as I'm always handing down a message to the new throw
> >     routine.
> >
> >     Hoping for a review :)
> >
> >     Best regards
> >     Christoph
> >
> >     > -----Original Message-----
> >     > From: Kenji Kazumura [mailto:kzr at jp.fujitsu.com]
> >     > Sent: Mittwoch, 8. Juni 2016 02:51
> >     > To: Langer, Christoph <christoph.langer at sap.com>
> >     > Cc: net-dev at openjdk.java.net <mailto:net-dev at openjdk.java.net>;
> >     nio-dev at openjdk.java.net <mailto:nio-dev at openjdk.java.net>; core-libs-
> >     > dev at openjdk.java.net <mailto:dev at openjdk.java.net>
> >     > Subject: Re: RFR 8158023: SocketExceptions contain too little
> >     information
> >     > sometimes
> >     >
> >     > Christoph,
> >     >
> >     > You should not remove conversion codes (platform string to Java
> >     String)
> >     > at JNU_ThrowByNameWithLastError,
> >     > and you have to add conversion codes at
> >     > JNU_ThrowByNameWithMessageAndLastError.
> >     >
> >     > It seems that you assume strerror returns only ascii characters,
> >     but actually
> >     > not.
> >     > It depends on the locale of your environment where java programs
> >     runs.
> >     >
> >     >
> >     > -Kenji Kazumura
> >     >
> >     >
> >     > In message
> >     >
> <decc19cdab854bbeac7126cb8e236f1e at DEWDFE13DE11.global.corp.sap
> >
> <mailto:decc19cdab854bbeac7126cb8e236f1e at DEWDFE13DE11.global.corp.sa
> p>>
> >     >    RFR 8158023: SocketExceptions contain too little information
> >     sometimes
> >     >    "Langer, Christoph"
> >     <<mailto:christoph.langer at sap.com>christoph.langer at sap.com> wrote:
> >     >
> >     > > Hi all,
> >     > >
> >     > > please review the following change:
> >     > > Webrev:
> >
> <http://cr.openjdk.java.net/%7Eclanger/webrevs/8158023.1/>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
> >     > >
> >