WSAGetLastError usage in jdk/src/java.base/windows/native/libnet/SocketInputStream.c

[Thomas Stüfe]

Hi Matthias,

not strictly necessary, since WSAGetLastError() only refers to socket
calls and GetIntField() is unlikely to call socket functions...
However, I think your proposal is fine for code cleanliness sake.

Best Regards, Thomas

On Tue, Aug 28, 2018 at 2:13 PM, Baesken, Matthias
<matthias.baesken at sap.com> wrote:
> Hello,
>
> the MSDN docu about WSAGetLastError warns to get the error-code
> ***immediately*** after occurance.
>
> See :
>
>
>
> https://msdn.microsoft.com/de-de/library/windows/desktop/ms741580(v=vs.85).aspx
>
>
>
> " ... If a function call's return value indicates that error or other
> relevant data was returned in the error code,
>
> WSAGetLastError should be called immediately ..."
>
>
>
> However in windows SocketInputStream.c , this was not done; we noticed very
> seldom errors because of this (not reproducible however) so
>
> we had a fix for this in our code base for a long time.
>
>
>
> Should we change this as well in OpenJDK , for example from :
>
>
>
> jdk/src/java.base/windows/native/libnet/SocketInputStream.c
>
>
>
>
>
> 120    nread = recv(fd, bufP, len, 0);
>
> 121    if (nread > 0) {
>
> 122        (*env)->SetByteArrayRegion(env, data, off, nread, (jbyte *)bufP);
>
> 123    } else {
>
> 124        if (nread < 0) {
>
> 125            // Check if the socket has been closed since we last checked.
>
> 126            // This could be a reason for recv failing.
>
> 127            if ((*env)->GetIntField(env, fdObj, IO_fd_fdID) == -1) {
>
> 128                JNU_ThrowByName(env, "java/net/SocketException", "Socket
> closed");
>
> 129            } else {
>
> 130                switch (WSAGetLastError()) {
>
>
>
> to  :
>
>
>
> 120    nread = recv(fd, bufP, len, 0);
>
> 121    if (nread > 0) {
>
> 122        (*env)->SetByteArrayRegion(env, data, off, nread, (jbyte *)bufP);
>
> 123    } else {
>
> 124        if (nread < 0) {
>
> 125            int err = WSAGetLastError();
>
> ...
>
>                    switch (err) {
>
>
>
>
>
> Thanks and best regards, Matthias