Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

2019-06-12 Thread Brian Burkhalter
Hi Ivan, > On Jun 12, 2019, at 4:03 PM, Ivan Gerasimov wrote: > > On 6/12/19 10:02 AM, Brian Burkhalter wrote: >> Actually, never mind, I am being completely lame here: both NET_ThrowNew() >> and the Windows function LocalFree() are robust to a NULL-valued buf so I >> think we can just remove

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

2019-06-12 Thread Ivan Gerasimov
On 6/12/19 10:02 AM, Brian Burkhalter wrote: Actually, never mind, I am being completely lame here: both NET_ThrowNew() and the Windows function LocalFree() are robust to a NULL-valued buf so I think we can just remove the n > 0 or buf == NULL check altogether. That's true, assuming that you

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

2019-06-12 Thread Brian Burkhalter
Actually, never mind, I am being completely lame here: both NET_ThrowNew() and the Windows function LocalFree() are robust to a NULL-valued buf so I think we can just remove the n > 0 or buf == NULL check altogether. Sorry for the noise: I should have checked this first. Thanks, Brian > On Ju

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

2019-06-12 Thread Brian Burkhalter
Hi Ivan, I am perhaps beating a dead horse here, but how about this instead? if (n > 0) { NET_ThrowNew(env, err, buf); LocalFree(buf); } else { NET_ThrowNew(env, err, "FormatMessage failed");

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

2019-06-11 Thread Ivan Gerasimov
Thanks Brian! The webrev looks fine to me. I think that *most likely* the check if (buf != NULL) will work as expected: buf will only be set to non-NULL value upon success. However, the documentation for the function FormatMessage states: """ If the function fails, the return value is zero.

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

2019-06-11 Thread Brian Burkhalter
Hi Ivan, I updated the patch: http://cr.openjdk.java.net/~bpb/8223813/webrev.01/ Please see comments inline below. > On Jun 11, 2019, at 5:06 PM, Ivan Gerasimov wrote: > > Inet4AddressImpl.c: > > 1) There is an extra space after FormatMess

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

2019-06-11 Thread Ivan Gerasimov
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

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

2019-06-11 Thread Brian Burkhalter
Hi Roger, Thanks for the $0.02. > On Jun 11, 2019, at 12:14 PM, Roger Riggs wrote: > > Having the extraneous suffix consistently removed seems like a good thing. > Though I'm not sure what the best form of the utility function is: > 1) return the count of characters to remove > 2) just trunca

Re: 8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

2019-06-11 Thread Roger Riggs
Hi Brian, Having the extraneous suffix consistently removed seems like a good thing. Though I'm not sure what the best form of the utility function is:  1) return the count of characters to remove  2) just truncate the buffer. There may be ways to force some of the errors that would lead to the

8223813: (aio) Iocp.getErrorMessage should drop trailing \r\n

2019-06-11 Thread Brian Burkhalter
https://bugs.openjdk.java.net/browse/JDK-8223813 http://cr.openjdk.java.net/~bpb/8223813/webrev.00/ FormatMessage() and FormatMessageW() occur in a number of locations: src/java.base/windows/