Hi Matthias,
The fix looks good to me.
Thank you for catching and fixing this!
Thanks,
Serguei
On 9/13/19 3:01 AM, Baesken, Matthias wrote:
Hello , my colleague Ralf pointed out that the NULL-check of the
result of GetStringUTFChars
should be done right after the GetStringUTFChars so I moved the
NULL-check up :
http://cr.openjdk.java.net/~mbaesken/webrevs/8230901.2/
<http://cr.openjdk.java.net/%7Embaesken/webrevs/8230901.2/>
Best regards, Matthias
Hi Thomas, thanks for the review .
You are correct about atoi .
New webrev :
http://cr.openjdk.java.net/~mbaesken/webrevs/8230901.1/
<http://cr.openjdk.java.net/%7Embaesken/webrevs/8230901.1/>
I had 2 additional observations :
1. With OJDK on solaris 32bit gone for quite some time, we might be
able to kick out the whole non _LP64 code because we are
always 64 bit
(maybe someone could comment if this is a safe assumption, there
might be old 32bit solaris core files flying around for some reason
even these days … )
http://cr.openjdk.java.net/~mbaesken/webrevs/8230901.1/src/jdk.hotspot.agent/solaris/native/libsaproc/saproc.cpp.frames.html
<http://cr.openjdk.java.net/%7Embaesken/webrevs/8230901.1/src/jdk.hotspot.agent/solaris/native/libsaproc/saproc.cpp.frames.html>
696 // some older versions of libproc.so crash when trying to attach
32 bit
697 // debugger to 64 bit core file. check and throw error.
698 #ifndef _LP64
…..
2. The usage of atoi is commented here :
https://docs.oracle.com/cd/E86824_01/html/E54766/atoi-3c.html
„However, applications should not use the atoi(), atol(), or
atoll() functions unless they know the value represented by the
argument will be in range for the corresponding result type”
……And here :
https://pubs.opengroup.org/onlinepubs/009695399/functions/atoi.html
“If the number is not known to be in range, /strtol/()
<https://pubs.opengroup.org/onlinepubs/009695399/functions/strtol.html> should
be used because /atoi/() is not required to perform any error checking”
However we have a number of usages in the coding where atoi is
called without knowing that the argument is in the allowed range .
some examples :
src/hotspot/share/runtime/arguments.cpp-382- if (match_option(option,
"-Dsun.java.launcher.pid=", &tail)) {
src/hotspot/share/runtime/arguments.cpp:383: _sun_java_launcher_pid =
atoi(tail);
src/hotspot/share/runtime/arguments.cpp-384- continue;
src/java.desktop/unix/native/libawt_xawt/xawt/XToolkit.c
455 value = getenv("_AWT_MAX_POLL_TIMEOUT");
456 if (value != NULL) {
457 AWT_MAX_POLL_TIMEOUT = atoi(value);
src/java.desktop/unix/native/common/awt/X11Color.c-781- if
(getenv("CMAPSIZE") != 0) {
src/java.desktop/unix/native/common/awt/X11Color.c:782: cmapsize =
atoi(getenv("CMAPSIZE"));
Should I open a bug for these ?
Best regards, Matthias
