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

[serguei.spitsyn at oracle.com]

Hi Matthias,

LGTM++

Thanks,
Serguei


On 2/25/20 11:30, Alex Menkov wrote:
> Hi Matthias,
>
> LGTM
>
> --alex
>
> On 02/25/2020 08:20, Baesken, Matthias wrote:
>>
>> New webrev :
>>
>>
>> http://cr.openjdk.java.net/~mbaesken/webrevs/8239462.3/
>>
>>
>> Best regards, Matthias
>>
>>
>>> IMO the solution with goto makes it even worse.
>>> If you don't want to introduce the wrapper, could you please restore
>>> changes in LinuxDebuggerLocal_attach0 from webrev.1
>>>
>>> --alex
>>>
>>> On 02/21/2020 00:32, Baesken, Matthias wrote:
>>>> Hi Alex ,
>>>>
>>>> new webrev :
>>>>
>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8239462.2/
>>>>
>>>> Best Regards, Matthias
>>>>
>>>>
>>>>>
>>>>> 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
>>>>>
>>>>