Oops, I missed the reply from Roger Riggs, which already covered many of these concerns.

Sincerely yours,
Ivan

On 23.09.2015 16:14, Ivan Gerasimov wrote:
Hi Rob!

On 21.09.2015 18:53, Rob McKenna wrote:
Hi folks,

Requesting a review of this change which switches corelibs usages of the thread-unsafe strerror over to strerror_r/strerror_s:

http://cr.openjdk.java.net/~robm/8133249/webrev.01/


I think it would be better to have jdk_strerror(errno, tmpbuf, sizeof(tmpbuf)) instead of jdk_strerror(errno, tmpbuf, (size_t) 1024), just for the sake of avoiding constant duplication. I don't see the return code of jdk_strerror() is ever used. I suggest using it in ProcessImpl_md.c (see below), or just making this function return void.

jni_util_md.c:
- why can't we just call jdk_strerror(errno, buf, len) and get rid of the temp buffer?

java_md_common.c:
- commented out #include @ line 26
- the change in JLI_ReportErrorMessageSys() looks incomplete

ProcessImpl_md.c:
- would it make sense to check if jdk_strerror() == EINVAL instead of comparing the strings?

PlainDatagramSocketImpl.c:
- #include added, but no other changes were made.

TwoStacksPlainDatagramSocketImpl.c:
- buf is declared to be char[1024], but only up to 255 chars are used.
- would be better to move the buf declaration closer to its use, maybe next to char errmsg[300] line?

jdk_strerror.c:
- minor nit: extra space after strerror_r at lines 46 and 63.

Sincerely yours,
Ivan



Reply via email to