Re: RFR 8158023: SocketExceptions contain too little information sometimes

2016-06-29 Thread Roger Riggs

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

RE: RFR 8158023: SocketExceptions contain too little information sometimes

2016-06-29 Thread Langer, Christoph
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-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

2016-06-28 Thread Langer, Christoph
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/



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/

   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<mailto:k...@jp.fujitsu.com>]
   > Sent: Mittwoch, 8. Juni 2016 02:51
   > To: Langer, Christoph 
<christoph.lan...@sap.com<mailto: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.sap>>
   >RFR 8158023: SocketExceptions contain too little information sometimes
   >"Langer, Christoph" 
<christoph.lan...@sap.com<mailto:christoph.lan...@sap.com>> wrote:
   >
   > > 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
   > >





RE: RFR 8158023: SocketExceptions contain too little information sometimes

2016-06-27 Thread Langer, Christoph
Hi Mark,

thanks for looking at this. Please see my comments inline.

>  wrt JNU_ThrowByNameWithMessaheAndLastError, it would appear that it
> doesn't allow for malloc to fail and hence
> str1 could be null  and a problematic input to jio_snprintf which could
> result in a de-referencing of zero.

You are right, thanks. I should check for null and return "OutOfMemory" 
appropriately. I fixed that and updated the webrev in place.

> in the call flow would it not be more appropriate to manipulate native
> buffers first to produce the desired error string and
> then allocate the java string object?
> c.f. src/java.base/unic/native/libnet/net_util_md.c
> NET_ThrowUnknownHostExceptionWithGaiError

Well, in the case of NET_ThrowUnknownHostExceptionWithGaiError you will 
probably have the hostname as well as the error text encoded in platform 
encoding. So for that it is perfectly fine to do the native buffers first and 
then convert to a Java String. But in my case for 
JNU_ThrowByNameWithMessageAndLastError, the errno text will be in platform 
encoding but the other message and formatting will be UTF-8 strings from the 
executable. So I have to handle both parts differently when creating a String 
object out of it. Or am I wrong in my assumption here?

> should you allow for getLastErrorString not to return an error string or
> return an error result?
> as in JNU_ThrowByNameWithLastError

I do, don't I? I'm checking the return value of getLastErrorString: "if (n > 
0)".

Best regards
Christoph

> 
> On 27/06/2016 08:42, Langer, Christoph wrote:
> > Hi,
> >
> > eventually here is the - hopefully final - version of this fix:
> > http://cr.openjdk.java.net/~clanger/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; nio-...@openjdk.java.net; core-libs-
> >> 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>
> >> RFR 8158023: SocketExceptions contain too little information sometimes
> >> "Langer, Christoph" <christoph.lan...@sap.com> wrote:
> >>
> >>> 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
> >>>



Re: RFR 8158023: SocketExceptions contain too little information sometimes

2016-06-27 Thread Mark Sheppard

Hi,
wrt JNU_ThrowByNameWithMessaheAndLastError, it would appear that it 
doesn't allow for malloc to fail and hence
str1 could be null  and a problematic input to jio_snprintf which could 
result in a de-referencing of zero.


in the call flow would it not be more appropriate to manipulate native 
buffers first to produce the desired error string and

then allocate the java string object?
c.f. src/java.base/unic/native/libnet/net_util_md.c
NET_ThrowUnknownHostExceptionWithGaiError

should you allow for getLastErrorString not to return an error string or 
return an error result?

as in JNU_ThrowByNameWithLastError

regards
Mark

On 27/06/2016 08:42, Langer, Christoph wrote:

Hi,

eventually here is the - hopefully final - version of this fix:
http://cr.openjdk.java.net/~clanger/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; nio-...@openjdk.java.net; core-libs-
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>
RFR 8158023: SocketExceptions contain too little information sometimes
"Langer, Christoph" <christoph.lan...@sap.com> wrote:


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





RE: RFR 8158023: SocketExceptions contain too little information sometimes

2016-06-27 Thread Langer, Christoph
Hi,

eventually here is the - hopefully final - version of this fix:
http://cr.openjdk.java.net/~clanger/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; nio-...@openjdk.java.net; core-libs-
> 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>
>RFR 8158023: SocketExceptions contain too little information sometimes
>"Langer, Christoph" <christoph.lan...@sap.com> wrote:
> 
> > 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
> >


Re: RFR 8158023: SocketExceptions contain too little information sometimes

2016-06-08 Thread Kenji Kazumura
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 
   RFR 8158023: SocketExceptions contain too little information sometimes
   "Langer, Christoph"  wrote:

> 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
> 


RE: RFR 8158023: SocketExceptions contain too little information sometimes

2016-06-08 Thread Langer, Christoph
Hi Kenji,

thanks for this very good point. I thought that JNU_ThrowByNameWithLastError 
just tries to throw the exception manually and if that fails it calls 
JNU_ThrowByName which basically does the same. But I missed the point that it 
does the conversion and tries JNU_ThrowByName only as fallback. So I'll 
revert/correct that. But now I think there are other places in the JDK which 
potentially miss a conversion of strerror result when they go through 
JNU_ThrowIOException for instance. I'll check that.

Please note that the current thread of discussion of my change is this one:
http://mail.openjdk.java.net/pipermail/net-dev/2016-June/009880.html

There is an updated webrev but it obviously still contains this mistake.

Thanks
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; nio-...@openjdk.java.net; core-libs-
> 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>
>RFR 8158023: SocketExceptions contain too little information sometimes
>"Langer, Christoph" <christoph.lan...@sap.com> wrote:
>
> > 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
> >