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-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-...@openjdk.java.net; net-dev@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-dev@openjdk.java.net <mailto:net-dev@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 > > > > > >
