Hi Christph,

fine, push when ready.

On 6/29/2016 5:18 PM, Langer, Christoph wrote:
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.
I misread line 216, thinking the exception class was fixed.

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.
Yep,

Thanks, Roger


Thanks
Christoph

-----Original Message-----
From: nio-dev [mailto:nio-dev-boun...@openjdk.java.net] On Behalf Of Roger
Riggs
Sent: Mittwoch, 29. Juni 2016 20:20
To: nio-...@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:pbened...@apache.org]
*Sent:* Montag, 27. Juni 2016 18:15
*To:* Langer, Christoph <christoph.lan...@sap.com>
*Cc:* Kenji Kazumura <k...@jp.fujitsu.com>; Chris Hegarty
<chris.hega...@oracle.com>; nio-...@openjdk.java.net;
core-libs-dev@openjdk.java.net; net-...@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.lan...@sap.com <mailto:christoph.lan...@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:k...@jp.fujitsu.com]
     > Sent: Mittwoch, 8. Juni 2016 02:51
     > To: Langer, Christoph <christoph.lan...@sap.com>
     > Cc: net-...@openjdk.java.net <mailto:net-...@openjdk.java.net>;
     nio-...@openjdk.java.net <mailto:nio-...@openjdk.java.net>; core-libs-
     > d...@openjdk.java.net <mailto:d...@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
     >
<decc19cdab854bbeac7126cb8e236...@dewdfe13de11.global.corp.sap
<mailto:decc19cdab854bbeac7126cb8e236...@dewdfe13de11.global.corp.sa
p>>
     >    RFR 8158023: SocketExceptions contain too little information
     sometimes
     >    "Langer, Christoph"
     <<mailto:christoph.lan...@sap.com>christoph.lan...@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
     > >


Reply via email to