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.
"""
So, my preference would be if (n > 0).
With kind regards,
Ivan
On 6/11/19 5:26 PM, Brian Burkhalter wrote:
Hi Ivan,
I updated the patch:
http://cr.openjdk.java.net/~bpb/8223813/webrev.01/
<http://cr.openjdk.java.net/%7Ebpb/8223813/webrev.01/>
Please see comments inline below.
On Jun 11, 2019, at 5:06 PM, Ivan Gerasimov
<ivan.gerasi...@oracle.com <mailto:ivan.gerasi...@oracle.com>> wrote:
Inet4AddressImpl.c:
1) There is an extra space after FormatMessage,
Fixed.
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).
I put “if (buf != NULL)” instead of “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 :)
So changed.
Everything else looks good to me.
Thanks for the comments!
Brian
--
With kind regards,
Ivan Gerasimov
