Hi Christoph,
thanks for the response ... yes, you do check the
getLastErrorString return ... sorry about that!
regards
Mark
On 27/06/2016 14:28, Langer, Christoph wrote:
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-dev@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
