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
