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

[Baesken, Matthias]

> not strictly necessary, since WSAGetLastError() only refers to socket calls

I'll try to find out more about this.

> However, I think your proposal is fine for code cleanliness sake.

Yes , then I will do it for code cleanliness sake (and because MS recommends anyway to get the error immediately).

When looking  at the   WSAGetLastError()   usages in the whole JDK  Windows  code I found 2 strange  usages ,   
  Both times   it looks like  WSAGetLastError()   is used to   get the error of the malloc call ,  in case this really works,  
  WSAGetLastError  might indeed alias in some way to GetLastError :

jdk/src/java.base/windows/native/libnet/Inet6AddressImpl.c 

384static jboolean
385ping6(JNIEnv *env, HANDLE hIcmpFile, SOCKETADDRESS *sa,
386      SOCKETADDRESS *netif, jint timeout)
387{
..............
396    ReplyBuffer = (VOID *)malloc(ReplySize);
397    if (ReplyBuffer == NULL) {
398        IcmpCloseHandle(hIcmpFile);
399        NET_ThrowNew(env, WSAGetLastError(), "Unable to allocate memory");
400        return JNI_FALSE;
401    }


jdk/src/java.base/windows/native/libnet/Inet4AddressImpl.c 

306static jboolean
307ping4(JNIEnv *env, HANDLE hIcmpFile, SOCKETADDRESS *sa,
308      SOCKETADDRESS *netif, jint timeout)
309{
.........
326    ReplyBuffer = (VOID *)malloc(ReplySize);
327    if (ReplyBuffer == NULL) {
328        IcmpCloseHandle(hIcmpFile);
329        NET_ThrowNew(env, WSAGetLastError(), "Unable to allocate memory");
330        return JNI_FALSE;
331    }


Best regards, Matthias




> -----Original Message-----
> From: Thomas Stüfe <thomas.stuefe at gmail.com>
> Sent: Dienstag, 28. August 2018 14:23
> To: Baesken, Matthias <matthias.baesken at sap.com>
> Cc: net-dev <net-dev at openjdk.java.net>
> Subject: Re: WSAGetLastError usage in
> jdk/src/java.base/windows/native/libnet/SocketInputStream.c
> 
> 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