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:[email protected]] On Behalf Of Roger > Riggs > Sent: Mittwoch, 29. Juni 2016 20:20 > To: [email protected] > 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:[email protected]] > > *Sent:* Montag, 27. Juni 2016 18:15 > > *To:* Langer, Christoph <[email protected]> > > *Cc:* Kenji Kazumura <[email protected]>; Chris Hegarty > > <[email protected]>; [email protected]; > > [email protected]; [email protected] > > *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 > > <[email protected] <mailto:[email protected]>> 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:[email protected]] > > > Sent: Mittwoch, 8. Juni 2016 02:51 > > > To: Langer, Christoph <[email protected]> > > > Cc: [email protected] <mailto:[email protected]>; > > [email protected] <mailto:[email protected]>; core-libs- > > > [email protected] <mailto:[email protected]> > > > 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 > > > > <[email protected] > > > <mailto:[email protected] > p>> > > > RFR 8158023: SocketExceptions contain too little information > > sometimes > > > "Langer, Christoph" > > <<mailto:[email protected]>[email protected]> 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 > > > > > >
