RFR [XS]: 8239462: jdk.hotspot.agent misses some ReleaseStringUTFChars calls in case of early returns

[Alex Menkov]

Hi Matthias,

Looks good in general, but I think it makes sense to fix #2 cases (at 
least I see them in LinuxDebuggerLocal). If GetStringUTFChars fails, the 
code will crash.
Also I see GetStringUTFChars(str, JNI_FALSE). This look bad as well - 
2nd arg is a pointer, so it should be NULL or nullptr.

As for #1 and #3 - AFAIU they are both right ways.
If GetStringUTFChars fails, it throws OOM and return NULL.

And one more thing to consider.
LinuxDebuggerLocal_attach0 function looks terrible - 7 
ReleaseStringUTFChars calls for 2 GetStringUTFChars.
Maybe it make sense to introduce simple wrapper like AutoJavaString in 
src/jdk.hotspot.agent/windows/native/libsaproc/sawindbg.cpp
It would make the code simpler and less error prone.

--alex

On 02/20/2020 00:53, Baesken, Matthias wrote:
> Hi Alex / Christoph,  thanks for the reviews.
> 
> New webrev :
> 
> http://cr.openjdk.java.net/~mbaesken/webrevs/8239462.1/
> 
> - includes  LinuxDebuggerLocal.cpp
> - adds a blank Christoph wanted to have
> 
> 
> A question (hopefully not a stupid one �� ):
> At most places in the coding,  GetStringUTFChars success is
> 1. handled by checking NULL , like this :
> 
> const char *s = (*env)->GetStringUTFChars(env, p, NULL);
> if (s == NULL) {
>     // handle failure
> }
> 
> 2.At some places , success / failure is not handled at all .
> 
> 3.Here (e.g.  LinuxDebuggerLocal.cpp)  success / failure check is done by
> 
>   if (env->ExceptionOccurred()) {  ... }
> 
> Which one is the best / right way to do    it (most likely not 2.) ?
> 
> 
> Best regards, Matthias
> 
> 
> 
>>
>> Looks like
>> src/jdk.hotspot.agent/linux/native/libsaproc/LinuxDebuggerLocal.cpp
>> has similar issues. It would be nice to fix them as well.
>>
>> --alex
>>
>> On 02/19/2020 06:21, Baesken, Matthias wrote:
>>> Hello, please review this small change .
>>> We miss at a few places ReleaseStringUTFChars  calls in the native
>>> jdk.hotspot.agent  coding.
>>> This happens in case of  early returns .
>>>
>>>
>>> Bug/webrev :
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8239462
>>>
>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8239462.0/
>>>
>>>
>>> Thanks, Matthias
>>>