8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n
Ivan Gerasimov
ivan.gerasimov at oracle.com
Wed Jun 12 00:06:58 UTC 2019
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/nio-dev/attachments/20190611/f0799ad7/attachment.html>
More information about the nio-dev
mailing list