Hi,

following up my change for https://bugs.openjdk.java.net/browse/JDK-8158023 I'd 
like to discuss the removal of NET_ThrowByNameWithLastError and its replacement 
with JNU_ThrowByNameWithLastError.

Currently NET_ThrowByNameWithLastError just adds the errno to a default message 
and then calls JNU_ThrowByNameWithLastError with that extended default message. 
However, the default message will only be printed out if an exception occurs in 
converting an error string to a Java String object or if instantiation of the 
new exception object will fail. So it is highly unlikely that the added error 
number will ever show up to a user. Furthermore, the sprintf inside 
NET_ThrowByNameWithLastError is prone to memory overwrites if e.g. the 
defaultDetail is too long and the errmsg buffer will be exceeded.

One could argue that the call to WSAGetLastError() for the Windows version 
could bring some benefits over the common JNU_ThrowByNameWithLastError function 
but not even that is the case as getLastErrorString() on Windows calls the 
Windows API GetLastError() and this has been the same in result as 
WSAGetLastError() for a very long time on Windows (I believe ever since the 
32bit era has begun with Windows 95 or so). And as stated before, this error 
number will only very unlikely show up, anyway.

To sume it up: The only added value of NET_ThrowByNameWithLastError is that 
there could be cases where you might see an error number value - though I think 
this is very unlikely to happen. But if you say this is valuable I suggest to 
add this error number fallback to JNU_ThrowByNameWithLastError in common.

Here is a webrev that would remove NET_ThrowByNameWithLastError: 
http://cr.openjdk.java.net/~clanger/webrevs/Remove_NET_ThrowByNameWithLastError/.
 But I did not yet file a bug for this...

Thanks in advance for your comments
Christoph

Reply via email to