RFR [XS]: 8230901: missing ReleaseStringUTFChars in servicability native code

[serguei.spitsyn at oracle.com]

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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20190913/4b55da27/attachment.html>