> 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.stu...@gmail.com> > Sent: Dienstag, 28. August 2018 14:23 > To: Baesken, Matthias <matthias.baes...@sap.com> > Cc: net-dev <net-dev@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.baes...@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