Hi Brian!
Inet4AddressImpl.c:
1) There is an extra space after FormatMessage,
2) It is preexisting. The code doesn't check if FormatMessage failed to
allocate the buffer.
It's not clear from the MSDN documentation, if the pointer to the buffer
will be set to NULL upon the failure.
If it does not, then subsequent NET_ThrowNew(env, err, buf);
LocalFree(buf); may hit uninitialized memory.
It would be more accurate to invoke them only if (n > 0).
3) It purely optional, but you may want to use the TEXT macro to append
the L prefix to the character literals, if TCHAR is defined to be WCHAR:
389 if (buf[n - 1] == TEXT('\n')) n--;
390 if (buf[n - 1] == TEXT('\r')) n--;
391 if (buf[n - 1] == TEXT('.')) n--;
392 buf[n] = TEXT('\0');
It may make the compiler just a tiny bit happier :)
Everything else looks good to me.
With kind regards,
Ivan
On 6/11/19 10:56 AM, Brian Burkhalter wrote:
https://bugs.openjdk.java.net/browse/JDK-8223813
http://cr.openjdk.java.net/~bpb/8223813/webrev.00/
<http://cr.openjdk.java.net/%7Ebpb/8223813/webrev.00/>
FormatMessage() and FormatMessageW() occur in a number of locations:
src/java.base/windows/native/libjli/java_md.c
src/java.base/windows/native/libnet/Inet4AddressImpl.c
src/java.base/windows/native/libjava/ProcessImpl_md.c
src/java.base/windows/native/libjava/jni_util_md.c
src/java.base/windows/native/libnio/ch/Iocp.c
src/java.base/windows/native/libnio/fs/WindowsNativeDispatcher.c
src/java.base/share/native/libzip/zlib/gzlib.c
Some of these already strip the terminal CRLF (or dot + CRLF) of the
string populated by FormatMessage[W](). This patch would add removing
them, if present, from
src/java.base/windows/native/libnet/Inet4AddressImpl.c
src/java.base/windows/native/libnio/ch/Iocp.c
src/java.base/windows/native/libnio/fs/WindowsNativeDispatcher.c
One question is whether it would be better just to consolidate this
code into two methods for example in jni_uitl and call the methods
from the other locations. There are already getLastErrorString() and
getErrorString() for chars here.
Also, I am not sure how to test this effectively. The code passes all
tiers as-is.
Thanks,
Brian
--
With kind regards,
Ivan Gerasimov
